If a user key gets negatively instantiated, an error code is cached in the
payload area. A negatively instantiated key may be then be positively
instantiated by updating it with valid data. However, the ->update key
type method must be aware that the error code may be there.
The following may be used to trigger the bug in the user key type:
keyctl request2 user user "" @u
keyctl add user user "a" @u
which manifests itself as:
BUG: unable to handle kernel paging request at 00000000ffffff8a
IP: [<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046
PGD 7cc30067 PUD 0
Oops: 0002 [#1] SMP
Modules linked in:
CPU: 3 PID: 2644 Comm: a.out Not tainted 4.3.0+ #49
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88003ddea700 ti: ffff88003dd88000 task.ti: ffff88003dd88000
RIP: 0010:[<ffffffff810a376f>] [<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280
[<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046
RSP: 0018:ffff88003dd8bdb0 EFLAGS: 00010246
RAX: 00000000ffffff82 RBX: 0000000000000000 RCX: 0000000000000001
RDX: ffffffff81e3fe40 RSI: 0000000000000000 RDI: 00000000ffffff82
RBP: ffff88003dd8bde0 R08: ffff88007d2d2da0 R09: 0000000000000000
R10: 0000000000000000 R11: ffff88003e8073c0 R12: 00000000ffffff82
R13: ffff88003dd8be68 R14: ffff88007d027600 R15: ffff88003ddea700
FS: 0000000000b92880(0063) GS:ffff88007fd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00000000ffffff8a CR3: 000000007cc5f000 CR4: 00000000000006e0
Stack:
ffff88003dd8bdf0 ffffffff81160a8a 0000000000000000 00000000ffffff82
ffff88003dd8be68 ffff88007d027600 ffff88003dd8bdf0 ffffffff810a39e5
ffff88003dd8be20 ffffffff812a31ab ffff88007d027600 ffff88007d027620
Call Trace:
[<ffffffff810a39e5>] kfree_call_rcu+0x15/0x20 kernel/rcu/tree.c:3136
[<ffffffff812a31ab>] user_update+0x8b/0xb0 security/keys/user_defined.c:129
[< inline >] __key_update security/keys/key.c:730
[<ffffffff8129e5c1>] key_create_or_update+0x291/0x440 security/keys/key.c:908
[< inline >] SYSC_add_key security/keys/keyctl.c:125
[<ffffffff8129fc21>] SyS_add_key+0x101/0x1e0 security/keys/keyctl.c:60
[<ffffffff8185f617>] entry_SYSCALL_64_fastpath+0x12/0x6a arch/x86/entry/entry_64.S:185
Note the error code (-ENOKEY) in EDX.
A similar bug can be tripped by:
keyctl request2 trusted user "" @u
keyctl add trusted user "a" @u
This should also affect encrypted keys - but that has to be correctly
parameterised or it will fail with EINVAL before getting to the bit that
will crashes.
Reported-by: Dmitry Vyukov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Mimi Zohar <[email protected]>
Signed-off-by: James Morris <[email protected]>
Normally, the transmit phase of a client call is implicitly ack'd by the
reception of the first data packet of the response being received.
However, if a security negotiation happens, the transmit phase, if it is
entirely contained in a single packet, may get an ack packet in response
and then may get aborted due to security negotiation failure.
Because the client has shifted state to RXRPC_CALL_CLIENT_AWAIT_REPLY due
to having transmitted all the data, the code that handles processing of the
received ack packet doesn't note the hard ack the data packet.
The following abort packet in the case of security negotiation failure then
incurs an assertion failure when it tries to drain the Tx queue because the
hard ack state is out of sync (hard ack means the packets have been
processed and can be discarded by the sender; a soft ack means that the
packets are received but could still be discarded and rerequested by the
receiver).
To fix this, we should record the hard ack we received for the ack packet.
The assertion failure looks like:
RxRPC: Assertion failed
1 <= 0 is false
0x1 <= 0x0 is false
------------[ cut here ]------------
kernel BUG at ../net/rxrpc/ar-ack.c:431!
...
RIP: 0010:[<ffffffffa006857b>] [<ffffffffa006857b>] rxrpc_rotate_tx_window+0xbc/0x131 [af_rxrpc]
...
Signed-off-by: David Howells <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
This fixes CVE-2015-5327. It affects kernels from 4.3-rc1 onwards.
Fix the X.509 time validation to use month number-1 when looking up the
number of days in that month. Also put the month number validation before
doing the lookup so as not to risk overrunning the array.
This can be tested by doing the following:
cat <<EOF | openssl x509 -outform DER | keyctl padd asymmetric "" @s
-----BEGIN CERTIFICATE-----
MIIDbjCCAlagAwIBAgIJAN/lUld+VR4hMA0GCSqGSIb3DQEBCwUAMCkxETAPBgNV
BAoMCGxvY2FsLWNhMRQwEgYDVQQDDAtzaWduaW5nIGtleTAeFw0xNTA5MDEyMTMw
MThaFw0xNjA4MzEyMTMwMThaMCkxETAPBgNVBAoMCGxvY2FsLWNhMRQwEgYDVQQD
DAtzaWduaW5nIGtleTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANrn
crcMfMeG67nagX4+m02Xk9rkmsMKI5XTUxbikROe7GSUVJ27sPVPZp4mgzoWlvhh
jfK8CC/qhEhwep8Pgg4EJZyWOjhZb7R97ckGvLIoUC6IO3FC2ZnR7WtmWDgo2Jcj
VlXwJdHhKU1VZwulh81O61N8IBKqz2r/kDhIWiicUCUkI/Do/RMRfKAoDBcSh86m
gOeIAGfq62vbiZhVsX5dOE8Oo2TK5weAvwUIOR7OuGBl5AqwFlPnXQolewiHzKry
THg9e44HfzG4Mi6wUvcJxVaQT1h5SrKD779Z5+8+wf1JLaooetcEUArvWyuxCU59
qxA4lsTjBwl4cmEki+cCAwEAAaOBmDCBlTAMBgNVHRMEBTADAQH/MAsGA1UdDwQE
AwIHgDAdBgNVHQ4EFgQUyND/eKUis7ep/hXMJ8iZMdUhI+IwWQYDVR0jBFIwUIAU
yND/eKUis7ep/hXMJ8iZMdUhI+KhLaQrMCkxETAPBgNVBAoMCGxvY2FsLWNhMRQw
EgYDVQQDDAtzaWduaW5nIGtleYIJAN/lUld+VR4hMA0GCSqGSIb3DQEBCwUAA4IB
AQAMqm1N1yD5pimUELLhT5eO2lRdGUfTozljRxc7e2QT3RLk2TtGhg65JFFN6eml
XS58AEPVcAsSLDlR6WpOpOLB2giM0+fV/eYFHHmh22yqTJl4YgkdUwyzPdCHNOZL
hmSKeY9xliHb6PNrNWWtZwhYYvRaO2DX4GXOMR0Oa2O4vaYu6/qGlZOZv3U6qZLY
wwHEJSrqeBDyMuwN+eANHpoSpiBzD77S4e+7hUDJnql4j6xzJ65+nWJ89fCrQypR
4sN5R3aGeIh3QAQUIKpHilwek0CtEaYERgc5m+jGyKSc1rezJW62hWRTaitOc+d5
G5hh+9YpnYcxQHEKnZ7rFNKJ
-----END CERTIFICATE-----
EOF
If it works, it emit a key ID; if it fails, it should give a bad message
error.
Reported-by: Mimi Zohar <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-by: Mimi Zohar <[email protected]>
Acked-by: David Woodhouse <[email protected]>
Signed-off-by: James Morris <[email protected]>
Handle a write being requested to the page immediately beyond the EOF
marker on a cache object. Currently this gets an assertion failure in
CacheFiles because the EOF marker is used there to encode information about
a partial page at the EOF - which could lead to an unknown blank spot in
the file if we extend the file over it.
The problem is actually in fscache where we check the index of the page
being written against store_limit. store_limit is set to the number of
pages that we're allowed to store by fscache_set_store_limit() - which
means it's one more than the index of the last page we're allowed to store.
The problem is that we permit writing to a page with an index _equal_ to
the store limit - when we should reject that case.
Whilst we're at it, change the triggered assertion in CacheFiles to just
return -ENOBUFS instead.
The assertion failure looks something like this:
CacheFiles: Assertion failed
1000 < 7b1 is false
------------[ cut here ]------------
kernel BUG at fs/cachefiles/rdwr.c:962!
...
RIP: 0010:[<ffffffffa02c9e83>] [<ffffffffa02c9e83>] cachefiles_write_page+0x273/0x2d0 [cachefiles]
Cc: [email protected] # v2.6.31+; earlier - that + backport of a17754f (at least)
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
The supplied script takes a signed module file and extracts the tailmost
signature (there could theoretically be more than one) and dumps all or
part of it or the unsigned file to stdout.
Call as:
scripts/extract-module-sig.pl -[0adnks] module-file >out
where the initial flag indicates which bit of the signed file you want dumping
to stdout:
(*) "-0". Dumps the unsigned data with the signature stripped.
(*) "-a". Dumps all of the signature data, including the magic number.
(*) "-d". Dumps the signature information block as a sequence of decimal
numbers in text form with spaces between (crypto algorithm type,
hash type, identifier type, signer's name length, key identifier
length and signature length).
(*) "-n". Dumps the signer's name contents.
(*) "-k". Dumps the key identifier contents.
(*) "-s". Dumps the cryptographic signature contents.
In the case that the signature is a PKCS#7 (or CMS) message, -n and -k will
print a warning to stderr and dump nothing to stdout, but will otherwise
complete okay; the entire PKCS#7/CMS message will be dumped by "-s"; and "-d"
will show "0 0 2 0 0 <pkcs#7-msg-len>".
Signed-off-by: David Howells <[email protected]>
The supplied script takes a vmlinux file - and if necessary a System.map
file - locates the system certificates list and extracts it to the named
file.
Call as:
./scripts/extract-sys-certs vmlinux certs
if vmlinux contains symbols and:
./scripts/extract-sys-certs -s System.map vmlinux certs
if it does not.
It prints something like the following to stdout:
Have 27 sections
No symbols in vmlinux, trying System.map
Have 80088 symbols
Have 1346 bytes of certs at VMA 0xffffffff8201c540
Certificate list in section .init.data
Certificate list at file offset 0x141c540
If vmlinux contains symbols then that is used rather than System.map - even
if one is given.
Signed-off-by: David Howells <[email protected]>
Don't #include crypto/public_key.h in akcipher as the contents of the
header aren't used and changes in a future patch cause it to fail to
compile if CONFIG_KEYS=n.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
If request_key() is used to find a keyring, only do the search part - don't
do the construction part if the keyring was not found by the search. We
don't really want keyrings in the negative instantiated state since the
rejected/negative instantiation error value in the payload is unioned with
keyring metadata.
Now the kernel gives an error:
request_key("keyring", "#selinux,bdekeyring", "keyring", KEY_SPEC_USER_SESSION_KEYRING) = -1 EPERM (Operation not permitted)
Signed-off-by: David Howells <[email protected]>
The following sequence of commands:
i=`keyctl add user a a @s`
keyctl request2 keyring foo bar @t
keyctl unlink $i @s
tries to invoke an upcall to instantiate a keyring if one doesn't already
exist by that name within the user's keyring set. However, if the upcall
fails, the code sets keyring->type_data.reject_error to -ENOKEY or some
other error code. When the key is garbage collected, the key destroy
function is called unconditionally and keyring_destroy() uses list_empty()
on keyring->type_data.link - which is in a union with reject_error.
Subsequently, the kernel tries to unlink the keyring from the keyring names
list - which oopses like this:
BUG: unable to handle kernel paging request at 00000000ffffff8a
IP: [<ffffffff8126e051>] keyring_destroy+0x3d/0x88
...
Workqueue: events key_garbage_collector
...
RIP: 0010:[<ffffffff8126e051>] keyring_destroy+0x3d/0x88
RSP: 0018:ffff88003e2f3d30 EFLAGS: 00010203
RAX: 00000000ffffff82 RBX: ffff88003bf1a900 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 000000003bfc6901 RDI: ffffffff81a73a40
RBP: ffff88003e2f3d38 R08: 0000000000000152 R09: 0000000000000000
R10: ffff88003e2f3c18 R11: 000000000000865b R12: ffff88003bf1a900
R13: 0000000000000000 R14: ffff88003bf1a908 R15: ffff88003e2f4000
...
CR2: 00000000ffffff8a CR3: 000000003e3ec000 CR4: 00000000000006f0
...
Call Trace:
[<ffffffff8126c756>] key_gc_unused_keys.constprop.1+0x5d/0x10f
[<ffffffff8126ca71>] key_garbage_collector+0x1fa/0x351
[<ffffffff8105ec9b>] process_one_work+0x28e/0x547
[<ffffffff8105fd17>] worker_thread+0x26e/0x361
[<ffffffff8105faa9>] ? rescuer_thread+0x2a8/0x2a8
[<ffffffff810648ad>] kthread+0xf3/0xfb
[<ffffffff810647ba>] ? kthread_create_on_node+0x1c2/0x1c2
[<ffffffff815f2ccf>] ret_from_fork+0x3f/0x70
[<ffffffff810647ba>] ? kthread_create_on_node+0x1c2/0x1c2
Note the value in RAX. This is a 32-bit representation of -ENOKEY.
The solution is to only call ->destroy() if the key was successfully
instantiated.
Reported-by: Dmitry Vyukov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-by: Dmitry Vyukov <[email protected]>
In ovl_copy_up_locked(), newdentry is leaked if the function exits through
out_cleanup as this just to out after calling ovl_cleanup() - which doesn't
actually release the ref on newdentry.
The out_cleanup segment should instead exit through out2 as certainly
newdentry leaks - and possibly upper does also, though this isn't caught
given the catch of newdentry.
Without this fix, something like the following is seen:
BUG: Dentry ffff880023e9eb20{i=f861,n=#ffff880023e82d90} still in use (1) [unmount of tmpfs tmpfs]
BUG: Dentry ffff880023ece640{i=0,n=bigfile} still in use (1) [unmount of tmpfs tmpfs]
when unmounting the upper layer after an error occurred in copyup.
An error can be induced by creating a big file in a lower layer with
something like:
dd if=/dev/zero of=/lower/a/bigfile bs=65536 count=1 seek=$((0xf000))
to create a large file (4.1G). Overlay an upper layer that is too small
(on tmpfs might do) and then induce a copy up by opening it writably.
Reported-by: Ulrich Obergfell <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
Cc: <[email protected]> # v3.18+
Open the lower file with O_LARGEFILE in ovl_copy_up().
Pass O_LARGEFILE unconditionally in ovl_copy_up_data() as it's purely for
catching 32-bit userspace dealing with a file large enough that it'll be
mishandled if the application isn't aware that there might be an integer
overflow. Inside the kernel, there shouldn't be any problems.
Reported-by: Ulrich Obergfell <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
Cc: <[email protected]> # v3.18+
The sign-file.c program actually uses CMS rather than PKCS#7 to sign a file
since that allows the target X.509 certificate to be specified by
subjectKeyId rather than by issuer + serialNumber.
However, older versions of the OpenSSL crypto library (such as may be found
in CentOS 5.11) don't support CMS. Assume everything prior to
OpenSSL-1.0.0 doesn't support CMS and switch to using PKCS#7 in that case.
Further, the pre-1.0.0 OpenSSL only supports PKCS#7 signing with SHA1, so
give an error from the sign-file script if the caller requests anything
other than SHA1.
The compiler gives the following error with an OpenSSL crypto library
that's too old:
HOSTCC scripts/sign-file
scripts/sign-file.c:23:25: fatal error: openssl/cms.h: No such file or directory
#include <openssl/cms.h>
Reported-by: Vinson Lee <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: David Woodhouse <[email protected]>
Don't strip leading zeros from the crypto key ID when using it to construct
the struct key description as the signature in kernels up to and including
4.2 matched this aspect of the key. This means that 1 in 256 keys won't
actually match if their key ID begins with 00.
The key ID is stored in the module signature as binary and so must be
converted to text in order to invoke request_key() - but it isn't stripped
at this point.
Something like this is likely to be observed in dmesg when the key is loaded:
[ 1.572423] Loaded X.509 cert 'Build time autogenerated kernel
key: 62a7c3d2da278be024da4af8652c071f3fea33'
followed by this when we try and use it:
[ 1.646153] Request for unknown module key 'Build time autogenerated
kernel key: 0062a7c3d2da278be024da4af8652c071f3fea33' err -11
The 'Loaded' line should show an extra '00' on the front of the hex string.
This problem should not affect 4.3-rc1 and onwards because there the key
should be matched on one of its auxiliary identities rather than the key
struct's description string.
Reported-by: Arjan van de Ven <[email protected]>
Reported-by: Andy Whitcroft <[email protected]>
Signed-off-by: David Howells <[email protected]>
Remove headers #included unnecessarily from extract-cert.c lest they cause
compilation of the tool to fail against an older OpenSSL library.
Signed-off-by: David Howells <[email protected]>
Acked-by: David Woodhouse <[email protected]>
There appears to be a race between:
(1) key_gc_unused_keys() which frees key->security and then calls
keyring_destroy() to unlink the name from the name list
(2) find_keyring_by_name() which calls key_permission(), thus accessing
key->security, on a key before checking to see whether the key usage is 0
(ie. the key is dead and might be cleaned up).
Fix this by calling ->destroy() before cleaning up the core key data -
including key->security.
Reported-by: Petr Matousek <[email protected]>
Signed-off-by: David Howells <[email protected]>
Fix the following warning when compiling extract-cert:
scripts/extract-cert.c: In function `write_cert':
scripts/extract-cert.c:89:2: warning: format not a string literal and no format arguments [-Wformat-security]
ERR(!i2d_X509_bio(wb, x509), cert_dst);
^
whereby the ERR() macro is taking cert_dst as the format string. "%s"
should be used as the format string as the path could contain special
characters.
Signed-off-by: David Howells <[email protected]>
Reported-by: Jim Davis <[email protected]>
Acked-by : David Woodhouse <[email protected]>
Cc: James Morris <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Add OIDs for sha224, sha284 and sha512 hash algos and use them to select
the hashing algorithm. Without this, something like the following error
might get written to dmesg:
[ 31.829322] PKCS7: Unknown OID: [32] 2.16.840.1.101.3.4.2.3
[ 31.829328] PKCS7: Unknown OID: [180] 2.16.840.1.101.3.4.2.3
[ 31.829330] Unsupported digest algo: 55
Where the 55 on the third line is OID__NR indicating an unknown OID.
Reported-by: Valdis Kletnieks <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-By: Valdis Kletnieks <[email protected]>
Signed-off-by: James Morris <[email protected]>
The module signing script (sign-file) used to be a wrapper around the
openssl program. It has now been replaced by a C program that uses the
crypto library from the OpenSSL package meaning that the OpenSSL devel
packages are necessary to provide the devel library link and the header
files.
This would be openssl-devel on Fedora and libssl-dev on Debian.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Stephen Rothwell <[email protected]>
Signed-off-by: James Morris <[email protected]>
Move certificate handling out of the kernel/ directory and into a certs/
directory to get all the weird stuff in one place and move the generated
signing keys into this directory.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Fix the following warning:
scripts/sign-file.c: In function ‘main’:
scripts/sign-file.c:188: warning: value computed is not used
whereby the result of BIO_ctrl() is cast inside of BIO_reset() to an
integer of a different size - which we're not checking but probably should.
Reported-by: James Morris <[email protected]>
Signed-off-by: David Howells <[email protected]>
Add a MODULE_LICENSE() line to the PKCS#7 test key module to fix this
warning:
WARNING: modpost: missing MODULE_LICENSE() in
crypto/asymmetric_keys/pkcs7_test_key.o
Whilst we're at it, also add a module description.
Reported-by: James Morris <[email protected]>
Signed-off-by: David Howells <[email protected]>
The revised sign-file program is no longer a script that wraps the openssl
program, but now rather a program that makes use of OpenSSL's crypto
library. This means that to build the sign-file program, the kernel build
process now has a dependency on the OpenSSL development packages in
addition to OpenSSL itself.
Document this in Kconfig and in module-signing.txt.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
A PKCS#7 or CMS message can have per-signature authenticated attributes
that are digested as a lump and signed by the authorising key for that
signature. If such attributes exist, the content digest isn't itself
signed, but rather it is included in a special authattr which then
contributes to the signature.
Further, we already require the master message content type to be
pkcs7_signedData - but there's also a separate content type for the data
itself within the SignedData object and this must be repeated inside the
authattrs for each signer [RFC2315 9.2, RFC5652 11.1].
We should really validate the authattrs if they exist or forbid them
entirely as appropriate. To this end:
(1) Alter the PKCS#7 parser to reject any message that has more than one
signature where at least one signature has authattrs and at least one
that does not.
(2) Validate authattrs if they are present and strongly restrict them.
Only the following authattrs are permitted and all others are
rejected:
(a) contentType. This is checked to be an OID that matches the
content type in the SignedData object.
(b) messageDigest. This must match the crypto digest of the data.
(c) signingTime. If present, we check that this is a valid, parseable
UTCTime or GeneralTime and that the date it encodes fits within
the validity window of the matching X.509 cert.
(d) S/MIME capabilities. We don't check the contents.
(e) Authenticode SP Opus Info. We don't check the contents.
(f) Authenticode Statement Type. We don't check the contents.
The message is rejected if (a) or (b) are missing. If the message is
an Authenticode type, the message is rejected if (e) is missing; if
not Authenticode, the message is rejected if (d) - (f) are present.
The S/MIME capabilities authattr (d) unfortunately has to be allowed
to support kernels already signed by the pesign program. This only
affects kexec. sign-file suppresses them (CMS_NOSMIMECAP).
The message is also rejected if an authattr is given more than once or
if it contains more than one element in its set of values.
(3) Add a parameter to pkcs7_verify() to select one of the following
restrictions and pass in the appropriate option from the callers:
(*) VERIFYING_MODULE_SIGNATURE
This requires that the SignedData content type be pkcs7-data and
forbids authattrs. sign-file sets CMS_NOATTR. We could be more
flexible and permit authattrs optionally, but only permit minimal
content.
(*) VERIFYING_FIRMWARE_SIGNATURE
This requires that the SignedData content type be pkcs7-data and
requires authattrs. In future, this will require an attribute
holding the target firmware name in addition to the minimal set.
(*) VERIFYING_UNSPECIFIED_SIGNATURE
This requires that the SignedData content type be pkcs7-data but
allows either no authattrs or only permits the minimal set.
(*) VERIFYING_KEXEC_PE_SIGNATURE
This only supports the Authenticode SPC_INDIRECT_DATA content type
and requires at least an SpcSpOpusInfo authattr in addition to the
minimal set. It also permits an SPC_STATEMENT_TYPE authattr (and
an S/MIME capabilities authattr because the pesign program doesn't
remove these).
(*) VERIFYING_KEY_SIGNATURE
(*) VERIFYING_KEY_SELF_SIGNATURE
These are invalid in this context but are included for later use
when limiting the use of X.509 certs.
(4) The pkcs7_test key type is given a module parameter to select between
the above options for testing purposes. For example:
echo 1 >/sys/module/pkcs7_test_key/parameters/usage
keyctl padd pkcs7_test foo @s </tmp/stuff.pkcs7
will attempt to check the signature on stuff.pkcs7 as if it contains a
firmware blob (1 being VERIFYING_FIRMWARE_SIGNATURE).
Suggested-by: Andy Lutomirski <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Marcel Holtmann <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Make the X.509 ASN.1 time object decoder fill in a time64_t rather than a
struct tm to make comparison easier (unfortunately, this makes readable
display less easy) and export it so that it can be used by the PKCS#7 code
too.
Further, tighten up its parsing to reject invalid dates (eg. weird
characters, non-existent hour numbers) and unsupported dates (eg. timezones
other than 'Z' or dates earlier than 1970).
Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Make sign-file use the OpenSSL CMS routines to generate a message to be
used as the signature blob instead of the PKCS#7 routines. This allows us
to change how the matching X.509 certificate is selected. With PKCS#7 the
only option is to match on the serial number and issuer fields of an X.509
certificate; with CMS, we also have the option of matching by subjectKeyId
extension. The new behaviour is selected with the "-k" flag.
Without the -k flag specified, the output is pretty much identical to the
PKCS#7 output.
Whilst we're at it, don't include the S/MIME capability list in the message
as it's irrelevant to us.
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]
Since CMS is an evolution of PKCS#7, with much of the ASN.1 being
compatible, add support for CMS signed-data messages also [RFC5652 sec 5].
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
The key identifiers fabricated from an X.509 certificate are currently:
(A) Concatenation of serial number and issuer
(B) Concatenation of subject and subjectKeyID (SKID)
When verifying one X.509 certificate with another, the AKID in the target
can be used to match the authoritative certificate. The AKID can specify
the match in one or both of two ways:
(1) Compare authorityCertSerialNumber and authorityCertIssuer from the AKID
to identifier (A) above.
(2) Compare keyIdentifier from the AKID plus the issuer from the target
certificate to identifier (B) above.
When verifying a PKCS#7 message, the only available comparison is between
the IssuerAndSerialNumber field and identifier (A) above.
However, a subsequent patch adds CMS support. Whilst CMS still supports a
match on IssuerAndSerialNumber as for PKCS#7, it also supports an
alternative - which is the SubjectKeyIdentifier field. This is used to
match to an X.509 certificate on the SKID alone. No subject information is
available to be used.
To this end change the fabrication of (B) above to be from the X.509 SKID
alone. The AKID in keyIdentifier form then only matches on that and does
not include the issuer.
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
We only support PKCS#7 signed-data [RFC2315 sec 9] content at the top level,
so reject anything else. Further, check that the version numbers in
SignedData and SignerInfo are 1 in both cases.
Note that we don't restrict the inner content type. In the PKCS#7 code we
don't parse the data attached there, but merely verify the signature over
it.
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
Extract the function that drives the PKCS#7 signature verification given a
data blob and a PKCS#7 blob out from the module signing code and lump it with
the system keyring code as it's generic. This makes it independent of module
config options and opens it to use by the firmware loader.
Signed-off-by: David Howells <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Seth Forshee <[email protected]>
Cc: Kyle McMartin <[email protected]>
system_keyring.c doesn't need to #include module-internal.h as it doesn't use
the one thing that exports. Remove the inclusion.
Signed-off-by: David Howells <[email protected]>
Move to using PKCS#7 messages as module signatures because:
(1) We have to be able to support the use of X.509 certificates that don't
have a subjKeyId set. We're currently relying on this to look up the
X.509 certificate in the trusted keyring list.
(2) PKCS#7 message signed information blocks have a field that supplies the
data required to match with the X.509 certificate that signed it.
(3) The PKCS#7 certificate carries fields that specify the digest algorithm
used to generate the signature in a standardised way and the X.509
certificates specify the public key algorithm in a standardised way - so
we don't need our own methods of specifying these.
(4) We now have PKCS#7 message support in the kernel for signed kexec purposes
and we can make use of this.
To make this work, the old sign-file script has been replaced with a program
that needs compiling in a previous patch. The rules to build it are added
here.
Signed-off-by: David Howells <[email protected]>
Tested-by: Vivek Goyal <[email protected]>
Provide a utility that:
(1) Digests a module using the specified hash algorithm (typically sha256).
[The digest can be dumped into a file by passing the '-d' flag]
(2) Generates a PKCS#7 message that:
(a) Has detached data (ie. the module content).
(b) Is signed with the specified private key.
(c) Refers to the specified X.509 certificate.
(d) Has an empty X.509 certificate list.
[The PKCS#7 message can be dumped into a file by passing the '-p' flag]
(3) Generates a signed module by concatenating the old module, the PKCS#7
message, a descriptor and a magic string. The descriptor contains the
size of the PKCS#7 message and indicates the id_type as PKEY_ID_PKCS7.
(4) Either writes the signed module to the specified destination or renames
it over the source module.
This allows module signing to reuse the PKCS#7 handling code that was added
for PE file parsing for signed kexec.
Note that the utility is written in C and must be linked against the OpenSSL
crypto library.
Note further that I have temporarily dropped support for handling externally
created signatures until we can work out the best way to do those. Hopefully,
whoever creates the signature can give me a PKCS#7 certificate.
Signed-off-by: David Howells <[email protected]>
Tested-by: Vivek Goyal <[email protected]>
It is possible for a PKCS#7 message to have detached data. However, to verify
the signatures on a PKCS#7 message, we have to be able to digest the data.
Provide a function to supply that data. An error is given if the PKCS#7
message included embedded data.
This is used in a subsequent patch to supply the data to module signing where
the signature is in the form of a PKCS#7 message with detached data, whereby
the detached data is the module content that is signed.
Signed-off-by: David Howells <[email protected]>
Tested-by: Vivek Goyal <[email protected]>
If an X.509 certificate has an AuthorityKeyIdentifier extension that provides
an issuer and serialNumber, then make it so that these are used in preference
to the keyIdentifier field also held therein for searching for the signing
certificate.
If both the issuer+serialNumber and the keyIdentifier are supplied, then the
certificate is looked up by the former but the latter is checked as well. If
the latter doesn't match the subjectKeyIdentifier of the parent certificate,
EKEYREJECTED is returned.
This makes it possible to chain X.509 certificates based on the issuer and
serialNumber fields rather than on subjectKeyIdentifier. This is necessary as
we are having to deal with keys that are represented by X.509 certificates
that lack a subjectKeyIdentifier.
Signed-off-by: David Howells <[email protected]>
Tested-by: Vivek Goyal <[email protected]>
Extract both parts of the AuthorityKeyIdentifier, not just the keyIdentifier,
as the second part can be used to match X.509 certificates by issuer and
serialNumber.
Signed-off-by: David Howells <[email protected]>
Tested-by: Vivek Goyal <[email protected]>
Copy string names to tokens in ASN.1 compiler rather than storing a pointer
into the source text. This means we don't have to use "%*.*s" all over the
place.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Add an ASN.1 compiler option to dump the element tree to stdout.
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
An ANY object in an ASN.1 grammar that is marked OPTIONAL should be skipped
if there is no more data to be had.
This can be tested by editing X.509 certificates or PKCS#7 messages to
remove the NULL from subobjects that look like the following:
SEQUENCE {
OBJECT(2a864886f70d01010b);
NULL();
}
This is an algorithm identifier plus an optional parameter.
The modified DER can be passed to one of:
keyctl padd asymmetric "" @s </tmp/modified.x509
keyctl padd pkcs7_test foo @s </tmp/modified.pkcs7
It should work okay with the patch and produce EBADMSG without.
Signed-off-by: David Howells <[email protected]>
Tested-by: Marcel Holtmann <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
If the ASN.1 decoder is asked to parse a sequence of objects, non-optional
matches get skipped if there's no more data to be had rather than a
data-overrun error being reported.
This is due to the code segment that decides whether to skip optional
matches (ie. matches that could get ignored because an element is marked
OPTIONAL in the grammar) due to a lack of data also skips non-optional
elements if the data pointer has reached the end of the buffer.
This can be tested with the data decoder for the new RSA akcipher algorithm
that takes three non-optional integers. Currently, it skips the last
integer if there is insufficient data.
Without the fix, #defining DEBUG in asn1_decoder.c will show something
like:
next_op: pc=0/13 dp=0/270 C=0 J=0
- match? 30 30 00
- TAG: 30 266 CONS
next_op: pc=2/13 dp=4/270 C=1 J=0
- match? 02 02 00
- TAG: 02 257
- LEAF: 257
next_op: pc=5/13 dp=265/270 C=1 J=0
- match? 02 02 00
- TAG: 02 3
- LEAF: 3
next_op: pc=8/13 dp=270/270 C=1 J=0
next_op: pc=11/13 dp=270/270 C=1 J=0
- end cons t=4 dp=270 l=270/270
The next_op line for pc=8/13 should be followed by a match line.
This is not exploitable for X.509 certificates by means of shortening the
message and fixing up the ASN.1 CONS tags because:
(1) The relevant records being built up are cleared before use.
(2) If the message is shortened sufficiently to remove the public key, the
ASN.1 parse of the RSA key will fail quickly due to a lack of data.
(3) Extracted signature data is either turned into MPIs (which cope with a
0 length) or is simpler integers specifying algoritms and suchlike
(which can validly be 0); and
(4) The AKID and SKID extensions are optional and their removal is handled
without risking passing a NULL to asymmetric_key_generate_id().
(5) If the certificate is truncated sufficiently to remove the subject,
issuer or serialNumber then the ASN.1 decoder will fail with a 'Cons
stack underflow' return.
This is not exploitable for PKCS#7 messages by means of removal of elements
from such a message from the tail end of a sequence:
(1) Any shortened X.509 certs embedded in the PKCS#7 message are survivable
as detailed above.
(2) The message digest content isn't used if it shows a NULL pointer,
similarly, the authattrs aren't used if that shows a NULL pointer.
(3) A missing signature results in a NULL MPI - which the MPI routines deal
with.
(4) If data is NULL, it is expected that the message has detached content and
that is handled appropriately.
(5) If the serialNumber is excised, the unconditional action associated
with it will pick up the containing SEQUENCE instead, so no NULL
pointer will be seen here.
If both the issuer and the serialNumber are excised, the ASN.1 decode
will fail with an 'Unexpected tag' return.
In either case, there's no way to get to asymmetric_key_generate_id()
with a NULL pointer.
(6) Other fields are decoded to simple integers. Shortening the message
to omit an algorithm ID field will cause checks on this to fail early
in the verification process.
This can also be tested by snipping objects off of the end of the ASN.1 stream
such that mandatory tags are removed - or even from the end of internal
SEQUENCEs. If any mandatory tag is missing, the error EBADMSG *should* be
produced. Without this patch ERANGE or ENOPKG might be produced or the parse
may apparently succeed, perhaps with ENOKEY or EKEYREJECTED being produced
later, depending on what gets snipped.
Just snipping off the final BIT_STRING or OCTET_STRING from either sample
should be a start since both are mandatory and neither will cause an EBADMSG
without the patches
Reported-by: Marcel Holtmann <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-by: Marcel Holtmann <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
In an ASN.1 description where there is a CHOICE construct that contains
elements with IMPLICIT tags that refer to constructed types, actions to be
taken on those elements should be conditional on the corresponding element
actually being matched. Currently, however, such actions are performed
unconditionally in the middle of processing the CHOICE.
For example, look at elements 'b' and 'e' here:
A ::= SEQUENCE {
CHOICE {
b [0] IMPLICIT B ({ do_XXXXXXXXXXXX_b }),
c [1] EXPLICIT C ({ do_XXXXXXXXXXXX_c }),
d [2] EXPLICIT B ({ do_XXXXXXXXXXXX_d }),
e [3] IMPLICIT C ({ do_XXXXXXXXXXXX_e }),
f [4] IMPLICIT INTEGER ({ do_XXXXXXXXXXXX_f })
}
} ({ do_XXXXXXXXXXXX_A })
B ::= SET OF OBJECT IDENTIFIER ({ do_XXXXXXXXXXXX_oid })
C ::= SET OF INTEGER ({ do_XXXXXXXXXXXX_int })
They each have an action (do_XXXXXXXXXXXX_b and do_XXXXXXXXXXXX_e) that
should only be processed if that element is matched.
The problem is that there's no easy place to hang the action off in the
subclause (type B for element 'b' and type C for element 'e') because
subclause opcode sequences can be shared.
To fix this, introduce a conditional action opcode(ASN1_OP_MAYBE_ACT) that
the decoder only processes if the preceding match was successful. This can
be seen in an excerpt from the output of the fixed ASN.1 compiler for the
above ASN.1 description:
[ 13] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // e
[ 14] = _tagn(CONT, CONS, 3),
[ 15] = _jump_target(45), // --> C
[ 16] = ASN1_OP_MAYBE_ACT,
[ 17] = _action(ACT_do_XXXXXXXXXXXX_e),
In this, if the op at [13] is matched (ie. element 'e' above) then the
action at [16] will be performed. However, if the op at [13] doesn't match
or is skipped because it is conditional and some previous op matched, then
the action at [16] will be ignored.
Note that to make this work in the decoder, the ASN1_OP_RETURN op must set
the flag to indicate that a match happened. This is necessary because the
_jump_target() seen above introduces a subclause (in this case an object of
type 'C') which is likely to alter the flag. Setting the flag here is okay
because to process a subclause, a match must have happened and caused a
jump.
This cannot be tested with the code as it stands, but rather affects future
code.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Fix the handling of CHOICE types in the ASN.1 compiler to make SEQUENCE and
SET elements in a CHOICE be correctly rendered as skippable and conditional
as appropriate.
For example, in the following ASN.1:
Foo ::= SEQUENCE { w1 INTEGER, w2 Bar, w3 OBJECT IDENTIFIER }
Bar ::= CHOICE {
x1 Seq1,
x2 [0] IMPLICIT OCTET STRING,
x3 Seq2,
x4 SET OF INTEGER
}
Seq1 ::= SEQUENCE { y1 INTEGER, y2 INTEGER, y3 INTEGER }
Seq2 ::= SEQUENCE { z1 BOOLEAN, z2 BOOLEAN, z3 BOOLEAN }
the output in foo.c generated by:
./scripts/asn1_compiler foo.asn1 foo.c foo.h
included:
// Bar
// Seq1
[ 4] = ASN1_OP_MATCH,
[ 5] = _tag(UNIV, CONS, SEQ),
...
[ 13] = ASN1_OP_COND_MATCH_OR_SKIP, // x2
[ 14] = _tagn(CONT, PRIM, 0),
// Seq2
[ 15] = ASN1_OP_MATCH,
[ 16] = _tag(UNIV, CONS, SEQ),
...
[ 24] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x4
[ 25] = _tag(UNIV, CONS, SET),
...
[ 27] = ASN1_OP_COND_FAIL,
as a result of the CHOICE - but this is wrong on lines 4 and 15 because
both of these should be skippable (one and only one of the four can be
picked) and the one on line 15 should also be conditional so that it is
ignored if anything before it matches.
After the patch, it looks like:
// Bar
// Seq1
[ 4] = ASN1_OP_MATCH_JUMP_OR_SKIP, // x1
[ 5] = _tag(UNIV, CONS, SEQ),
...
[ 7] = ASN1_OP_COND_MATCH_OR_SKIP, // x2
[ 8] = _tagn(CONT, PRIM, 0),
// Seq2
[ 9] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x3
[ 10] = _tag(UNIV, CONS, SEQ),
...
[ 12] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x4
[ 13] = _tag(UNIV, CONS, SET),
...
[ 15] = ASN1_OP_COND_FAIL,
where all four options are skippable and the second, third and fourth are
all conditional, as is the backstop at the end.
This hasn't been a problem so far because in the ASN.1 specs we have are
either using primitives or are using SET OF and SEQUENCE OF which are
handled correctly.
Whilst we're at it, also make sure that element labels get included in
comments in the output for elements that have complex types.
This cannot be tested with the code as it stands, but rather affects future
code.
Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
Create a common helper function to determine the label for a new inode.
This is then used by:
- may_create()
- selinux_dentry_init_security()
- selinux_inode_init_security()
This will change the behaviour of the functions slightly, bringing them
all into line.
Suggested-by: Stephen Smalley <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Stephen Smalley <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
Make file->f_path always point to the overlay dentry so that the path in
/proc/pid/fd is correct and to ensure that label-based LSMs have access to the
overlay as well as the underlay (path-based LSMs probably don't need it).
Using my union testsuite to set things up, before the patch I see:
[root@andromeda union-testsuite]# bash 5</mnt/a/foo107
[root@andromeda union-testsuite]# ls -l /proc/$$/fd/
...
lr-x------. 1 root root 64 Jun 5 14:38 5 -> /a/foo107
[root@andromeda union-testsuite]# stat /mnt/a/foo107
...
Device: 23h/35d Inode: 13381 Links: 1
...
[root@andromeda union-testsuite]# stat -L /proc/$$/fd/5
...
Device: 23h/35d Inode: 13381 Links: 1
...
After the patch:
[root@andromeda union-testsuite]# bash 5</mnt/a/foo107
[root@andromeda union-testsuite]# ls -l /proc/$$/fd/
...
lr-x------. 1 root root 64 Jun 5 14:22 5 -> /mnt/a/foo107
[root@andromeda union-testsuite]# stat /mnt/a/foo107
...
Device: 23h/35d Inode: 40346 Links: 1
...
[root@andromeda union-testsuite]# stat -L /proc/$$/fd/5
...
Device: 23h/35d Inode: 40346 Links: 1
...
Note the change in where /proc/$$/fd/5 points to in the ls command. It was
pointing to /a/foo107 (which doesn't exist) and now points to /mnt/a/foo107
(which is correct).
The inode accessed, however, is the lower layer. The union layer is on device
25h/37d and the upper layer on 24h/36d.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Call ovl_drop_write() earlier in ovl_dentry_open() before we call vfs_open()
as we've done the copy up for which we needed the freeze-write lock by that
point.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Statistics polling can not be done by lmedm04 driver's implementation of
M88RS2000/TS2020 because I2C messages stop the device's demuxer, so allow
polling for statistics to be suppressed in the ts2020 driver by setting
dont_poll in the ts2020_config struct.
Reported-by: Malcolm Priestley <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Malcolm Priestley <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Copy the loop_through setting from the ts2020_config struct to the internal
ts2020_priv struct so that it can actually be used.
Whilst we're at it, group the bitfields together in the same order in both
structs so that the compiler has a good chance to copy them in one go.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Provide a DVBv5 API signal strength. This is in units of 0.001 dBm rather
than a percentage.
>From Antti Palosaari's testing with a signal generator, it appears that the
gain calculated according to Montage's specification if negated is a
reasonable representation of the signal strength of the generator.
To this end:
(1) Polled statistic gathering needed to be implemented in the TS2020 driver.
This is done in the ts2020_stat_work() function.
(2) The calculated gain is placed as the signal strength in the
dtv_property_cache associated with the front end with the scale set to
FE_SCALE_DECIBEL.
(3) The DVBv3 format signal strength then needed to be calculated from the
signal strength stored in the dtv_property_cache rather than accessing
the value when ts2020_read_signal_strength() is called.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
The TS2020 and TS2022 tuners take an input from the demodulator indicating the
AGC setting on that component that is then used to influence the tuner's own
gain. This should be taken into account when calculating the gain and signal
strength.
Further, the existing TS2020 driver miscalculates the signal strength as the
result of its calculations can exceed the storage capacity of the 16-bit word
used to return it to userspace.
To this end:
(1) Add a callback function (->get_agc_pwm()) in the ts2020_config struct that
the tuner can call to get the AGC PWM value from the demodulator.
(2) Modify the TS2020 driver to calculate the gain according to Montage's
specification with the adjustment that we produce a negative value and
scale it to 0.001dB units (which is what the DVBv5 API will require):
(a) Callback to the demodulator to retrieve the AGC PWM value and then
turn that into Vagc for incorporation in the calculations. If the
callback is unset, assume a Vagc of 0.
(b) Calculate the tuner gain from a combination of Vagc and the tuner's RF
gain and baseband gain settings.
(3) Turn this into a percentage signal strength as per Montage's
specification for return to userspace with the DVBv3 API.
(4) Provide a function in the M88DS3103 demodulator driver that can be used to
get the AGC PWM value on behalf of the tuner.
(5) The ts2020_config.get_agc_pwm function should be set by the code that
stitches together the drivers for each card.
For the DVBSky cards that use the M88DS3103 with the TS2020 or the TS2022,
set the get_agc_pwm function to point to m88ds3103_get_agc_pwm.
I have tested this with a DVBSky S952 card which has an M88DS3103 and a TS2022.
Thanks to Montage for providing access to information about the workings of
these parts.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
ts2020_attach() allocates a variable pdata on the stack and then passes a
pointer to it to i2c_new_device() which stashes the pointer in persistent
structures.
Add a comment to the effect that this isn't actually an error because the
contents of the variable are only used in ts2020_probe() and this is only
called ts2020_attach()'s stack frame exists.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
In comments and in the documentation, the units of properties marked
with the FE_SCALE_DECIBEL scale are specified in terms of 1/1000 dB
or 0.0001 dB. This is inconsistent, however, as 1/1000 is 0.001,
not 0.0001.
Note that the v4l-utils divide the value by 1000 for the signal
strength suggesting that the 1/1000 is correct.
Settle on millidecibels, ie. 1/1000dB or 0.001dB.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Make use of d_backing_inode() in pathwalk to gain access to an
inode or dentry that's on a lower layer.
Signed-off-by: David Howells <[email protected]>
In the test manager, there are a number of if-statements with expressions of
the form !x == y that incur warnings with gcc-5 of the following form:
../crypto/testmgr.c: In function '__test_aead':
../crypto/testmgr.c:523:12: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
if (!ret == template[i].fail) {
^
By converting the 'fail' member of struct aead_testvec and struct
cipher_testvec to a bool, we can get rid of the warnings.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
In testmgr, struct pcomp_testvec takes a non-const 'params' field, which is
pointed to a const deflate_comp_params or deflate_decomp_params object. With
gcc-5 this incurs the following warnings:
In file included from ../crypto/testmgr.c:44:0:
../crypto/testmgr.h:28736:13: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
.params = &deflate_comp_params,
^
../crypto/testmgr.h:28748:13: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
.params = &deflate_comp_params,
^
../crypto/testmgr.h:28776:13: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
.params = &deflate_decomp_params,
^
../crypto/testmgr.h:28800:13: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
.params = &deflate_decomp_params,
^
Fix this by making the parameters pointer const and constifying the things
that use it.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
library helpers called by filesystem drivers on their own inodes
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
that's the bulk of filesystem drivers dealing with inodes of their own
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
... except where that code acts as a filesystem driver, rather than
working with dentries given to it.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
most of the ->d_inode uses there refer to the same inode IO would
go to, i.e. d_backing_inode()
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
socket inodes and sunrpc filesystems - inodes owned by that code
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
places where we are dealing with S_ISSOCK file creation/lookups.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
relayfs and tracefs are dealing with inodes of their own;
those two act as filesystem drivers
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Fix up some ->d_inode accesses in the chelsio driver.
(1) FILE_DATA() should just be replaced with file_inode().
(2) set_debugfs_file_size() should be removed and debugfs_create_file_size()
should be used to create the file.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Cachefiles should perform fs modifications (eg. vfs_unlink()) on the top layer
only and should not attempt to alter the lower layer.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
AF_UNIX sockets should call mknod on the top layer only and should not attempt
to modify the lower layer in a layered filesystem such as overlayfs.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Fix inconsistent use of file_inode() vs file->f_path.dentry->d_inode.
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Make pathwalk use d_is_reg() rather than S_ISREG() to determine whether to
honour O_TRUNC. Since this occurs after complete_walk(), the dentry type
field cannot change and the inode pointer cannot change as we hold a ref on
the dentry, so this should be safe.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Fix up debugfs to use d_is_dir(dentry) in place of
S_ISDIR(dentry->d_inode->i_mode).
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Where we have:
if (!dentry->d_inode || d_is_negative(dentry)) {
type constructions in pathwalk we should be able to eliminate the check of
d_inode and rely solely on the result of d_is_negative() or d_is_positive().
What we do have to take care to do is to read d_inode after calling a
d_is_xxx() typecheck function to get the barriering right.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Don't use d_inode as a variable name as it now masks a function name.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Impose ordering on accesses of d_inode and d_flags to avoid the need to do
this:
if (!dentry->d_inode || d_is_negative(dentry)) {
when this:
if (d_is_negative(dentry)) {
should suffice.
This check is especially problematic if a dentry can have its type field set
to something other than DENTRY_MISS_TYPE when d_inode is NULL (as in
unionmount).
What we really need to do is stick a write barrier between setting d_inode and
setting d_flags and a read barrier between reading d_flags and reading
d_inode.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Supply two functions to test whether a filesystem's own dentries are positive
or negative (d_really_is_positive() and d_really_is_negative()).
The problem is that the DCACHE_ENTRY_TYPE field of dentry->d_flags may be
overridden by the union part of a layered filesystem and isn't thus
necessarily indicative of the type of dentry.
Normally, this would involve a negative dentry (ie. ->d_inode == NULL) having
->d_layer.lower pointed to a lower layer dentry, DCACHE_PINNING_LOWER set and
the DCACHE_ENTRY_TYPE field set to something other than DCACHE_MISS_TYPE - but
it could also involve, say, a DCACHE_SPECIAL_TYPE being overridden to
DCACHE_WHITEOUT_TYPE if a 0,0 chardev is detected in the top layer.
However, inside a filesystem, when that fs is looking at its own dentries, it
probably wants to know if they are really negative or not - and doesn't care
about the fallthrough bits used by the union.
To this end, a filesystem should normally use d_really_is_positive/negative()
when looking at its own dentries rather than d_is_positive/negative() and
should use d_inode() to get at the inode.
Anyone looking at someone else's dentries (this includes pathwalk) should use
d_is_xxx() and d_backing_inode().
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Al Viro <[email protected]>
Use enum to represent table offsets rather than hard-coding numbers to avoid
problems with the numbers becoming out of sync with the table.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
There are a pair of nested loops inside m88ts2022_cmd() that use the same
index variable, but for different things. Split the variable.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Antti Palosaari <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Now that the retrieval operation may be disposed of by fscache_put_operation()
before we actually set the context, the retrieval-specific cleanup operation
can produce a NULL-pointer dereference when it tries to unconditionally clean
up the netfs context.
Given that it is expected that we'll get at least as far as the place where we
currently set the context pointer and it is unlikely we'll go through the
error handling paths prior to that point, retain the context right from the
point that the retrieval op is allocated.
Concomitant to this, we need to retain the cookie pointer in the retrieval op
also so that we can call the netfs to release its context in the release
method.
In addition, we might now get into fscache_release_retrieval_op() with the op
only initialised. To this end, set the operation to DEAD only after the
release method has been called and skip the n_pages test upon cleanup if the
op is still in the INITIALISED state.
Without these changes, the following oops might be seen:
BUG: unable to handle kernel NULL pointer dereference at 00000000000000b8
...
RIP: 0010:[<ffffffffa0089c98>] fscache_release_retrieval_op+0xae/0x100
...
Call Trace:
[<ffffffffa0088560>] fscache_put_operation+0x117/0x2e0
[<ffffffffa008b8f5>] __fscache_read_or_alloc_pages+0x351/0x3ac
[<ffffffffa00b761f>] __nfs_readpages_from_fscache+0x59/0xbf [nfs]
[<ffffffffa00b06c5>] nfs_readpages+0x10c/0x185 [nfs]
[<ffffffff81124925>] ? alloc_pages_current+0x119/0x13e
[<ffffffff810ee5fd>] ? __page_cache_alloc+0xfb/0x10a
[<ffffffff810f87f8>] __do_page_cache_readahead+0x188/0x22c
[<ffffffff810f8b3a>] ondemand_readahead+0x29e/0x2af
[<ffffffff810f8c92>] page_cache_sync_readahead+0x38/0x3a
[<ffffffff810ef337>] generic_file_read_iter+0x1a2/0x55a
[<ffffffffa00a9dff>] ? nfs_revalidate_mapping+0xd6/0x288 [nfs]
[<ffffffffa00a6a23>] nfs_file_read+0x49/0x70 [nfs]
[<ffffffff811363be>] new_sync_read+0x78/0x9c
[<ffffffff81137164>] __vfs_read+0x13/0x38
[<ffffffff8113721e>] vfs_read+0x95/0x121
[<ffffffff811372f6>] SyS_read+0x4c/0x8a
[<ffffffff81557a52>] system_call_fastpath+0x12/0x17
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Any time an incomplete operation is cancelled, the operation cancellation
function needs to be called to clean up. This is currently being passed
directly to some of the functions that might want to call it, but not all.
Instead, pass the cancellation method pointer to the fscache_operation_init()
and have that cache it in the operation struct. Further, plug in a dummy
cancellation handler if the caller declines to set one as this allows us to
call the function unconditionally (the extra overhead isn't worth bothering
about as we don't expect to be calling this typically).
The cancellation method must thence be called everywhere the CANCELLED state
is set. Note that we call it *before* setting the CANCELLED state such that
the method can use the old state value to guide its operation.
fscache_do_cancel_retrieval() needs moving higher up in the sources so that
the init function can use it now.
Without this, the following oops may be seen:
FS-Cache: Assertion failed
FS-Cache: 3 == 0 is false
------------[ cut here ]------------
kernel BUG at ../fs/fscache/page.c:261!
...
RIP: 0010:[<ffffffffa0089c1b>] fscache_release_retrieval_op+0x77/0x100
[<ffffffffa008853d>] fscache_put_operation+0x114/0x2da
[<ffffffffa008b8c2>] __fscache_read_or_alloc_pages+0x358/0x3b3
[<ffffffffa00b761f>] __nfs_readpages_from_fscache+0x59/0xbf [nfs]
[<ffffffffa00b06c5>] nfs_readpages+0x10c/0x185 [nfs]
[<ffffffff81124925>] ? alloc_pages_current+0x119/0x13e
[<ffffffff810ee5fd>] ? __page_cache_alloc+0xfb/0x10a
[<ffffffff810f87f8>] __do_page_cache_readahead+0x188/0x22c
[<ffffffff810f8b3a>] ondemand_readahead+0x29e/0x2af
[<ffffffff810f8c92>] page_cache_sync_readahead+0x38/0x3a
[<ffffffff810ef337>] generic_file_read_iter+0x1a2/0x55a
[<ffffffffa00a9dff>] ? nfs_revalidate_mapping+0xd6/0x288 [nfs]
[<ffffffffa00a6a23>] nfs_file_read+0x49/0x70 [nfs]
[<ffffffff811363be>] new_sync_read+0x78/0x9c
[<ffffffff81137164>] __vfs_read+0x13/0x38
[<ffffffff8113721e>] vfs_read+0x95/0x121
[<ffffffff811372f6>] SyS_read+0x4c/0x8a
[<ffffffff81557a52>] system_call_fastpath+0x12/0x17
The assertion is showing that the remaining number of pages (n_pages) is not 0
when the operation is being released.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Call fscache_put_operation() or a wrapper on any op that has gone through
fscache_operation_init() so that the accounting shown in /proc is done
correctly, specifically fscache_n_op_release.
fscache_put_operation() therefore now allows an op in the INITIALISED state as
well as in the CANCELLED and COMPLETE states.
Note that this means that an operation can get put that doesn't have its
->object pointer filled in, so anything that depends on the object needs to be
conditional in fscache_put_operation().
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Cancellation of an in-progress operation needs to update the relevant counters
and start any operations that are pending waiting on this one.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Count and display through /proc/fs/fscache/stats the number of initialised
operations.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Out of line fscache_operation_init() so that it can access internal FS-Cache
features, such as stats, in a later commit.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Currently, fscache_cancel_op() only cancels pending operations - attempts to
cancel in-progress operations are ignored. This leads to a problem in
fscache_wait_for_operation_activation() whereby the wait is terminated, but
the object has been killed.
The check at the end of the function now triggers because it's no longer
contingent on the cache having produced an I/O error since the commit that
fixed the logic error in fscache_object_is_dead().
The result of the check is that it tries to cancel the operation - but since
the object may not be pending by this point, the cancellation request may be
ignored - with the result that the the object is just put by the caller and
fscache_put_operation has an assertion failure because the operation isn't in
either the COMPLETE or the CANCELLED states.
To fix this, we permit in-progress ops to be cancelled under some
circumstances.
The bug results in an oops that looks something like this:
FS-Cache: fscache_wait_for_operation_activation() = -ENOBUFS [obj dead 3]
FS-Cache:
FS-Cache: Assertion failed
FS-Cache: 3 == 5 is false
------------[ cut here ]------------
kernel BUG at ../fs/fscache/operation.c:432!
...
RIP: 0010:[<ffffffffa0088574>] fscache_put_operation+0xf2/0x2cd
Call Trace:
[<ffffffffa008b92a>] __fscache_read_or_alloc_pages+0x2ec/0x3b3
[<ffffffffa00b761f>] __nfs_readpages_from_fscache+0x59/0xbf [nfs]
[<ffffffffa00b06c5>] nfs_readpages+0x10c/0x185 [nfs]
[<ffffffff81124925>] ? alloc_pages_current+0x119/0x13e
[<ffffffff810ee5fd>] ? __page_cache_alloc+0xfb/0x10a
[<ffffffff810f87f8>] __do_page_cache_readahead+0x188/0x22c
[<ffffffff810f8b3a>] ondemand_readahead+0x29e/0x2af
[<ffffffff810f8c92>] page_cache_sync_readahead+0x38/0x3a
[<ffffffff810ef337>] generic_file_read_iter+0x1a2/0x55a
[<ffffffffa00a9dff>] ? nfs_revalidate_mapping+0xd6/0x288 [nfs]
[<ffffffffa00a6a23>] nfs_file_read+0x49/0x70 [nfs]
[<ffffffff811363be>] new_sync_read+0x78/0x9c
[<ffffffff81137164>] __vfs_read+0x13/0x38
[<ffffffff8113721e>] vfs_read+0x95/0x121
[<ffffffff811372f6>] SyS_read+0x4c/0x8a
[<ffffffff81557a52>] system_call_fastpath+0x12/0x17
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
fscache_object_is_dead() returns true only if the object is marked dead and
the cache got an I/O error. This should be a logical OR instead. Since two
of the callers got split up into handling for separate subcases, expand the
other callers and kill the function. This is probably the right thing to do
anyway since one of the subcases isn't about the object at all, but rather
about the cache.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
When an object is being marked as no longer live, do this under the object
spinlock to prevent a race with operation submission targeted on that object.
The problem occurs due to the following pair of intertwined sequences when the
cache tries to create an object that would take it over the hard available
space limit:
NETFS INTERFACE
===============
(A) The netfs calls fscache_acquire_cookie(). object creation is deferred to
the object state machine and the netfs is allowed to continue.
OBJECT STATE MACHINE KTHREAD
============================
(1) The object is looked up on disk by fscache_look_up_object()
calling cachefiles_walk_to_object(). The latter finds that the
object is not yet represented on disk and calls
fscache_object_lookup_negative().
(2) fscache_object_lookup_negative() sets FSCACHE_COOKIE_NO_DATA_YET
and clears FSCACHE_COOKIE_LOOKING_UP, thus allowing the netfs to
start queuing read operations.
(B) The netfs calls fscache_read_or_alloc_pages(). This calls
fscache_wait_for_deferred_lookup() which sees FSCACHE_COOKIE_LOOKING_UP
become clear, allowing the read to begin.
(C) A read operation is set up and passed to fscache_submit_op() to deal
with.
(3) cachefiles_walk_to_object() calls cachefiles_has_space(), which
fails (or one of the file operations to create stuff fails).
cachefiles returns an error to fscache.
(4) fscache_look_up_object() transits to the LOOKUP_FAILURE state,
(5) fscache_lookup_failure() sets FSCACHE_OBJECT_LOOKED_UP and
FSCACHE_COOKIE_UNAVAILABLE and clears FSCACHE_COOKIE_LOOKING_UP
then transits to the KILL_OBJECT state.
(6) fscache_kill_object() clears FSCACHE_OBJECT_IS_LIVE in an attempt
to reject any further requests from the netfs.
(7) object->n_ops is examined and found to be 0.
fscache_kill_object() transits to the DROP_OBJECT state.
(D) fscache_submit_op() locks the object spinlock, sees if it can dispatch
the op immediately by calling fscache_object_is_active() - which fails
since FSCACHE_OBJECT_IS_AVAILABLE has not yet been set.
(E) fscache_submit_op() then tests FSCACHE_OBJECT_LOOKED_UP - which is set.
It then queues the object and increments object->n_ops.
(8) fscache_drop_object() releases the object and eventually
fscache_put_object() calls cachefiles_put_object() which suffers
an assertion failure here:
ASSERTCMP(object->fscache.n_ops, ==, 0);
Locking the object spinlock in step (6) around the clearance of
FSCACHE_OBJECT_IS_LIVE ensures that the the decision trees in
fscache_submit_op() and fscache_submit_exclusive_op() don't see the IS_LIVE
flag being cleared mid-decision: either the op is queued before step (7) - in
which case fscache_kill_object() will see n_ops>0 and will deal with the op -
or the op will be rejected.
This, combined with rejecting op submission if the target object is dying, fix
the problem.
The problem shows up as the following oops:
CacheFiles: Assertion failed
CacheFiles: 1 == 0 is false
------------[ cut here ]------------
kernel BUG at ../fs/cachefiles/interface.c:339!
...
RIP: 0010:[<ffffffffa014fd9c>] [<ffffffffa014fd9c>] cachefiles_put_object+0x2a4/0x301 [cachefiles]
...
Call Trace:
[<ffffffffa008674b>] fscache_put_object+0x18/0x21 [fscache]
[<ffffffffa00883e6>] fscache_object_work_func+0x3ba/0x3c9 [fscache]
[<ffffffff81054dad>] process_one_work+0x226/0x441
[<ffffffff81055d91>] worker_thread+0x273/0x36b
[<ffffffff81055b1e>] ? rescuer_thread+0x2e1/0x2e1
[<ffffffff81059b9d>] kthread+0x10e/0x116
[<ffffffff81059a8f>] ? kthread_create_on_node+0x1bb/0x1bb
[<ffffffff815579ac>] ret_from_fork+0x7c/0xb0
[<ffffffff81059a8f>] ? kthread_create_on_node+0x1bb/0x1bb
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Reject new operations that are being submitted against an object if that
object has failed its lookup or creation states or has been killed by the
cache backend for some other reason, such as having been culled.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
When submitting an operation, prefer to cancel the operation immediately
rather than queuing it for later processing if the object is marked as dying
(ie. the object state machine has reached the KILL_OBJECT state).
Whilst we're at it, change the series of related test_bit() calls into a
READ_ONCE() and bitwise-AND operators to reduce the number of load
instructions (test_bit() has a volatile address).
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Move fscache_report_unexpected_submission() up within operation.c so that it
can be called from fscache_submit_exclusive_op() too.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Handle VERSION Rx protocol packets. We should respond to a VERSION packet
with a string indicating the Rx version. This is a maximum of 64 characters
and is padded out to 65 chars with NUL bytes.
Note that other AFS clients use the version request as a NAT keepalive so we
need to handle it rather than returning an abort.
The standard formulation seems to be:
<project> <version> built <yyyy>-<mm>-<dd>
for example:
" OpenAFS 1.6.2 built 2013-05-07 "
(note the three extra spaces) as obtained with:
rxdebug grand.mit.edu -version
from the openafs package.
Signed-off-by: David Howells <[email protected]>
afs_send_empty_reply() doesn't require an iovec array with which to initialise
the msghdr, but can pass NULL instead.
Suggested-by: Al Viro <[email protected]>
Signed-off-by: David Howells <[email protected]>
Use iov_iter_count() in rxrpc_send_data() to get the remaining data length
instead of using the len argument as the len argument is now redundant.
Signed-off-by: David Howells <[email protected]>
Don't call skb_add_data() in rxrpc_send_data() if there's no data to copy and
also skip the calculations associated with it in such a case.
Signed-off-by: David Howells <[email protected]>
This commit:
commit af2b040e47
Author: Al Viro <[email protected]>
Date: Thu Nov 27 21:44:24 2014 -0500
Subject: rxrpc: switch rxrpc_send_data() to iov_iter primitives
incorrectly changes a do-while loop into a while loop in rxrpc_send_data().
Unfortunately, at least one pass through the loop is required - even if
there is no data - so that the packet the closes the send phase can be
sent if MSG_MORE is not set.
Signed-off-by: David Howells <[email protected]>
Count the number of objects that get culled by the cache backend and the
number of objects that the cache backend declines to instantiate due to lack
of space in the cache.
These numbers are made available through /proc/fs/fscache/stats
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steve Dickson <[email protected]>
Acked-by: Jeff Layton <[email protected]>
David Howells