14 Dec, 2020

4 commits


18 Oct, 2020

1 commit

  • A previous commit changed the notification mode from true/false to an
    int, allowing notify-no, notify-yes, or signal-notify. This was
    backwards compatible in the sense that any existing true/false user
    would translate to either 0 (on notification sent) or 1, the latter
    which mapped to TWA_RESUME. TWA_SIGNAL was assigned a value of 2.

    Clean this up properly, and define a proper enum for the notification
    mode. Now we have:

    - TWA_NONE. This is 0, same as before the original change, meaning no
    notification requested.
    - TWA_RESUME. This is 1, same as before the original change, meaning
    that we use TIF_NOTIFY_RESUME.
    - TWA_SIGNAL. This uses TIF_SIGPENDING/JOBCTL_TASK_WORK for the
    notification.

    Clean up all the callers, switching their 0/1/false/true to using the
    appropriate TWA_* mode for notifications.

    Fixes: e91b48162332 ("task_work: teach task_work_add() to do signal_wake_up()")
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Jens Axboe

    Jens Axboe
     

13 Oct, 2020

1 commit

  • Pull compat iovec cleanups from Al Viro:
    "Christoph's series around import_iovec() and compat variant thereof"

    * 'work.iov_iter' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
    security/keys: remove compat_keyctl_instantiate_key_iov
    mm: remove compat_process_vm_{readv,writev}
    fs: remove compat_sys_vmsplice
    fs: remove the compat readv/writev syscalls
    fs: remove various compat readv/writev helpers
    iov_iter: transparently handle compat iovecs in import_iovec
    iov_iter: refactor rw_copy_check_uvector and import_iovec
    iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c
    compat.h: fix a spelling error in

    Linus Torvalds
     

03 Oct, 2020

2 commits


24 Aug, 2020

1 commit

  • Replace the existing /* fall through */ comments and its variants with
    the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
    fall-through markings when it is the case.

    [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

    Signed-off-by: Gustavo A. R. Silva

    Gustavo A. R. Silva
     

12 Aug, 2020

1 commit


08 Aug, 2020

1 commit

  • As said by Linus:

    A symmetric naming is only helpful if it implies symmetries in use.
    Otherwise it's actively misleading.

    In "kzalloc()", the z is meaningful and an important part of what the
    caller wants.

    In "kzfree()", the z is actively detrimental, because maybe in the
    future we really _might_ want to use that "memfill(0xdeadbeef)" or
    something. The "zero" part of the interface isn't even _relevant_.

    The main reason that kzfree() exists is to clear sensitive information
    that should not be leaked to other future users of the same memory
    objects.

    Rename kzfree() to kfree_sensitive() to follow the example of the recently
    added kvfree_sensitive() and make the intention of the API more explicit.
    In addition, memzero_explicit() is used to clear the memory to make sure
    that it won't get optimized away by the compiler.

    The renaming is done by using the command sequence:

    git grep -w --name-only kzfree |\
    xargs sed -i 's/kzfree/kfree_sensitive/'

    followed by some editing of the kfree_sensitive() kerneldoc and adding
    a kzfree backward compatibility macro in slab.h.

    [akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h]
    [akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more]

    Suggested-by: Joe Perches
    Signed-off-by: Waiman Long
    Signed-off-by: Andrew Morton
    Acked-by: David Howells
    Acked-by: Michal Hocko
    Acked-by: Johannes Weiner
    Cc: Jarkko Sakkinen
    Cc: James Morris
    Cc: "Serge E. Hallyn"
    Cc: Joe Perches
    Cc: Matthew Wilcox
    Cc: David Rientjes
    Cc: Dan Carpenter
    Cc: "Jason A . Donenfeld"
    Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com
    Signed-off-by: Linus Torvalds

    Waiman Long
     

07 Aug, 2020

1 commit

  • Rationale:
    Reduces attack surface on kernel devs opening the links for MITM
    as HTTPS traffic is much harder to manipulate.

    Deterministic algorithm:
    For each file:
    If not .svg:
    For each line:
    If doesn't contain `\bxmlns\b`:
    For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
    If both the HTTP and HTTPS versions
    return 200 OK and serve the same content:
    Replace HTTP with HTTPS.

    Signed-off-by: Alexander A. Klimov
    Acked-by: John Johansen
    Signed-off-by: James Morris

    Alexander A. Klimov
     

14 Jun, 2020

1 commit

  • …git/dhowells/linux-fs

    Pull notification queue from David Howells:
    "This adds a general notification queue concept and adds an event
    source for keys/keyrings, such as linking and unlinking keys and
    changing their attributes.

    Thanks to Debarshi Ray, we do have a pull request to use this to fix a
    problem with gnome-online-accounts - as mentioned last time:

    https://gitlab.gnome.org/GNOME/gnome-online-accounts/merge_requests/47

    Without this, g-o-a has to constantly poll a keyring-based kerberos
    cache to find out if kinit has changed anything.

    [ There are other notification pending: mount/sb fsinfo notifications
    for libmount that Karel Zak and Ian Kent have been working on, and
    Christian Brauner would like to use them in lxc, but let's see how
    this one works first ]

    LSM hooks are included:

    - A set of hooks are provided that allow an LSM to rule on whether or
    not a watch may be set. Each of these hooks takes a different
    "watched object" parameter, so they're not really shareable. The
    LSM should use current's credentials. [Wanted by SELinux & Smack]

    - A hook is provided to allow an LSM to rule on whether or not a
    particular message may be posted to a particular queue. This is
    given the credentials from the event generator (which may be the
    system) and the watch setter. [Wanted by Smack]

    I've provided SELinux and Smack with implementations of some of these
    hooks.

    WHY
    ===

    Key/keyring notifications are desirable because if you have your
    kerberos tickets in a file/directory, your Gnome desktop will monitor
    that using something like fanotify and tell you if your credentials
    cache changes.

    However, we also have the ability to cache your kerberos tickets in
    the session, user or persistent keyring so that it isn't left around
    on disk across a reboot or logout. Keyrings, however, cannot currently
    be monitored asynchronously, so the desktop has to poll for it - not
    so good on a laptop. This facility will allow the desktop to avoid the
    need to poll.

    DESIGN DECISIONS
    ================

    - The notification queue is built on top of a standard pipe. Messages
    are effectively spliced in. The pipe is opened with a special flag:

    pipe2(fds, O_NOTIFICATION_PIPE);

    The special flag has the same value as O_EXCL (which doesn't seem
    like it will ever be applicable in this context)[?]. It is given up
    front to make it a lot easier to prohibit splice&co from accessing
    the pipe.

    [?] Should this be done some other way? I'd rather not use up a new
    O_* flag if I can avoid it - should I add a pipe3() system call
    instead?

    The pipe is then configured::

    ioctl(fds[1], IOC_WATCH_QUEUE_SET_SIZE, queue_depth);
    ioctl(fds[1], IOC_WATCH_QUEUE_SET_FILTER, &filter);

    Messages are then read out of the pipe using read().

    - It should be possible to allow write() to insert data into the
    notification pipes too, but this is currently disabled as the
    kernel has to be able to insert messages into the pipe *without*
    holding pipe->mutex and the code to make this work needs careful
    auditing.

    - sendfile(), splice() and vmsplice() are disabled on notification
    pipes because of the pipe->mutex issue and also because they
    sometimes want to revert what they just did - but one or more
    notification messages might've been interleaved in the ring.

    - The kernel inserts messages with the wait queue spinlock held. This
    means that pipe_read() and pipe_write() have to take the spinlock
    to update the queue pointers.

    - Records in the buffer are binary, typed and have a length so that
    they can be of varying size.

    This allows multiple heterogeneous sources to share a common
    buffer; there are 16 million types available, of which I've used
    just a few, so there is scope for others to be used. Tags may be
    specified when a watchpoint is created to help distinguish the
    sources.

    - Records are filterable as types have up to 256 subtypes that can be
    individually filtered. Other filtration is also available.

    - Notification pipes don't interfere with each other; each may be
    bound to a different set of watches. Any particular notification
    will be copied to all the queues that are currently watching for it
    - and only those that are watching for it.

    - When recording a notification, the kernel will not sleep, but will
    rather mark a queue as having lost a message if there's
    insufficient space. read() will fabricate a loss notification
    message at an appropriate point later.

    - The notification pipe is created and then watchpoints are attached
    to it, using one of:

    keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fds[1], 0x01);
    watch_mount(AT_FDCWD, "/", 0, fd, 0x02);
    watch_sb(AT_FDCWD, "/mnt", 0, fd, 0x03);

    where in both cases, fd indicates the queue and the number after is
    a tag between 0 and 255.

    - Watches are removed if either the notification pipe is destroyed or
    the watched object is destroyed. In the latter case, a message will
    be generated indicating the enforced watch removal.

    Things I want to avoid:

    - Introducing features that make the core VFS dependent on the
    network stack or networking namespaces (ie. usage of netlink).

    - Dumping all this stuff into dmesg and having a daemon that sits
    there parsing the output and distributing it as this then puts the
    responsibility for security into userspace and makes handling
    namespaces tricky. Further, dmesg might not exist or might be
    inaccessible inside a container.

    - Letting users see events they shouldn't be able to see.

    TESTING AND MANPAGES
    ====================

    - The keyutils tree has a pipe-watch branch that has keyctl commands
    for making use of notifications. Proposed manual pages can also be
    found on this branch, though a couple of them really need to go to
    the main manpages repository instead.

    If the kernel supports the watching of keys, then running "make
    test" on that branch will cause the testing infrastructure to spawn
    a monitoring process on the side that monitors a notifications pipe
    for all the key/keyring changes induced by the tests and they'll
    all be checked off to make sure they happened.

    https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git/log/?h=pipe-watch

    - A test program is provided (samples/watch_queue/watch_test) that
    can be used to monitor for keyrings, mount and superblock events.
    Information on the notifications is simply logged to stdout"

    * tag 'notifications-20200601' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
    smack: Implement the watch_key and post_notification hooks
    selinux: Implement the watch_key security hook
    keys: Make the KEY_NEED_* perms an enum rather than a mask
    pipe: Add notification lossage handling
    pipe: Allow buffers to be marked read-whole-or-error for notifications
    Add sample notification program
    watch_queue: Add a key/keyring notification facility
    security: Add hooks to rule on setting a watch
    pipe: Add general notification queue support
    pipe: Add O_NOTIFICATION_PIPE
    security: Add a hook for the point of notification insertion
    uapi: General notification queue definitions

    Linus Torvalds
     

10 Jun, 2020

1 commit

  • Convert comments that reference mmap_sem to reference mmap_lock instead.

    [akpm@linux-foundation.org: fix up linux-next leftovers]
    [akpm@linux-foundation.org: s/lockaphore/lock/, per Vlastimil]
    [akpm@linux-foundation.org: more linux-next fixups, per Michel]

    Signed-off-by: Michel Lespinasse
    Signed-off-by: Andrew Morton
    Reviewed-by: Vlastimil Babka
    Reviewed-by: Daniel Jordan
    Cc: Davidlohr Bueso
    Cc: David Rientjes
    Cc: Hugh Dickins
    Cc: Jason Gunthorpe
    Cc: Jerome Glisse
    Cc: John Hubbard
    Cc: Laurent Dufour
    Cc: Liam Howlett
    Cc: Matthew Wilcox
    Cc: Peter Zijlstra
    Cc: Ying Han
    Link: http://lkml.kernel.org/r/20200520052908.204642-13-walken@google.com
    Signed-off-by: Linus Torvalds

    Michel Lespinasse
     

05 Jun, 2020

3 commits

  • Merge yet more updates from Andrew Morton:

    - More MM work. 100ish more to go. Mike Rapoport's "mm: remove
    __ARCH_HAS_5LEVEL_HACK" series should fix the current ppc issue

    - Various other little subsystems

    * emailed patches from Andrew Morton : (127 commits)
    lib/ubsan.c: fix gcc-10 warnings
    tools/testing/selftests/vm: remove duplicate headers
    selftests: vm: pkeys: fix multilib builds for x86
    selftests: vm: pkeys: use the correct page size on powerpc
    selftests/vm/pkeys: override access right definitions on powerpc
    selftests/vm/pkeys: test correct behaviour of pkey-0
    selftests/vm/pkeys: introduce a sub-page allocator
    selftests/vm/pkeys: detect write violation on a mapped access-denied-key page
    selftests/vm/pkeys: associate key on a mapped page and detect write violation
    selftests/vm/pkeys: associate key on a mapped page and detect access violation
    selftests/vm/pkeys: improve checks to determine pkey support
    selftests/vm/pkeys: fix assertion in test_pkey_alloc_exhaust()
    selftests/vm/pkeys: fix number of reserved powerpc pkeys
    selftests/vm/pkeys: introduce powerpc support
    selftests/vm/pkeys: introduce generic pkey abstractions
    selftests: vm: pkeys: use the correct huge page size
    selftests/vm/pkeys: fix alloc_random_pkey() to make it really random
    selftests/vm/pkeys: fix assertion in pkey_disable_set/clear()
    selftests/vm/pkeys: fix pkey_disable_clear()
    selftests: vm: pkeys: add helpers for pkey bits
    ...

    Linus Torvalds
     
  • For kvmalloc'ed data object that contains sensitive information like
    cryptographic keys, we need to make sure that the buffer is always cleared
    before freeing it. Using memset() alone for buffer clearing may not
    provide certainty as the compiler may compile it away. To be sure, the
    special memzero_explicit() has to be used.

    This patch introduces a new kvfree_sensitive() for freeing those sensitive
    data objects allocated by kvmalloc(). The relevant places where
    kvfree_sensitive() can be used are modified to use it.

    Fixes: 4f0882491a14 ("KEYS: Avoid false positive ENOMEM error on key read")
    Suggested-by: Linus Torvalds
    Signed-off-by: Waiman Long
    Signed-off-by: Andrew Morton
    Reviewed-by: Eric Biggers
    Acked-by: David Howells
    Cc: Jarkko Sakkinen
    Cc: James Morris
    Cc: "Serge E. Hallyn"
    Cc: Joe Perches
    Cc: Matthew Wilcox
    Cc: David Rientjes
    Cc: Uladzislau Rezki
    Link: http://lkml.kernel.org/r/20200407200318.11711-1-longman@redhat.com
    Signed-off-by: Linus Torvalds

    Waiman Long
     
  • Pull keyring updates from David Howells:

    - Fix a documentation warning.

    - Replace a zero-length array with a flexible one

    - Make the big_key key type use ChaCha20Poly1305 and use the crypto
    algorithm directly rather than going through the crypto layer.

    - Implement the update op for the big_key type.

    * tag 'keys-next-20200602' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
    keys: Implement update for the big_key type
    security/keys: rewrite big_key crypto to use library interface
    KEYS: Replace zero-length array with flexible-array
    Documentation: security: core.rst: add missing argument

    Linus Torvalds
     

03 Jun, 2020

2 commits

  • Implement the ->update op for the big_key type.

    Signed-off-by: David Howells
    Acked-by: Jason A. Donenfeld

    David Howells
     
  • A while back, I noticed that the crypto and crypto API usage in big_keys
    were entirely broken in multiple ways, so I rewrote it. Now, I'm
    rewriting it again, but this time using the simpler ChaCha20Poly1305
    library function. This makes the file considerably more simple; the
    diffstat alone should justify this commit. It also should be faster,
    since it no longer requires a mutex around the "aead api object" (nor
    allocations), allowing us to encrypt multiple items in parallel. We also
    benefit from being able to pass any type of pointer, so we can get rid
    of the ridiculously complex custom page allocator that big_key really
    doesn't need.

    [DH: Change the select CRYPTO_LIB_CHACHA20POLY1305 to a depends on as
    select doesn't propagate and the build can end up with an =y dependending
    on some =m pieces.

    The depends on CRYPTO also had to be removed otherwise the configurator
    complains about a recursive dependency.]

    Cc: Andy Lutomirski
    Cc: Greg KH
    Cc: Linus Torvalds
    Cc: kernel-hardening@lists.openwall.com
    Reviewed-by: Eric Biggers
    Signed-off-by: Jason A. Donenfeld
    Signed-off-by: David Howells

    Jason A. Donenfeld
     

02 Jun, 2020

1 commit

  • Pull crypto updates from Herbert Xu:
    "API:
    - Introduce crypto_shash_tfm_digest() and use it wherever possible.
    - Fix use-after-free and race in crypto_spawn_alg.
    - Add support for parallel and batch requests to crypto_engine.

    Algorithms:
    - Update jitter RNG for SP800-90B compliance.
    - Always use jitter RNG as seed in drbg.

    Drivers:
    - Add Arm CryptoCell driver cctrng.
    - Add support for SEV-ES to the PSP driver in ccp"

    * 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6: (114 commits)
    crypto: hisilicon - fix driver compatibility issue with different versions of devices
    crypto: engine - do not requeue in case of fatal error
    crypto: cavium/nitrox - Fix a typo in a comment
    crypto: hisilicon/qm - change debugfs file name from qm_regs to regs
    crypto: hisilicon/qm - add DebugFS for xQC and xQE dump
    crypto: hisilicon/zip - add debugfs for Hisilicon ZIP
    crypto: hisilicon/hpre - add debugfs for Hisilicon HPRE
    crypto: hisilicon/sec2 - add debugfs for Hisilicon SEC
    crypto: hisilicon/qm - add debugfs to the QM state machine
    crypto: hisilicon/qm - add debugfs for QM
    crypto: stm32/crc32 - protect from concurrent accesses
    crypto: stm32/crc32 - don't sleep in runtime pm
    crypto: stm32/crc32 - fix multi-instance
    crypto: stm32/crc32 - fix run-time self test issue.
    crypto: stm32/crc32 - fix ext4 chksum BUG_ON()
    crypto: hisilicon/zip - Use temporary sqe when doing work
    crypto: hisilicon - add device error report through abnormal irq
    crypto: hisilicon - remove codes of directly report device errors through MSI
    crypto: hisilicon - QM memory management optimization
    crypto: hisilicon - unify initial value assignment into QM
    ...

    Linus Torvalds
     

19 May, 2020

2 commits

  • Since the meaning of combining the KEY_NEED_* constants is undefined, make
    it so that you can't do that by turning them into an enum.

    The enum is also given some extra values to represent special
    circumstances, such as:

    (1) The '0' value is reserved and causes a warning to trap the parameter
    being unset.

    (2) The key is to be unlinked and we require no permissions on it, only
    the keyring, (this replaces the KEY_LOOKUP_FOR_UNLINK flag).

    (3) An override due to CAP_SYS_ADMIN.

    (4) An override due to an instantiation token being present.

    (5) The permissions check is being deferred to later key_permission()
    calls.

    The extra values give the opportunity for LSMs to audit these situations.

    [Note: This really needs overhauling so that lookup_user_key() tells
    key_task_permission() and the LSM what operation is being done and leaves
    it to those functions to decide how to map that onto the available
    permits. However, I don't really want to make these change in the middle
    of the notifications patchset.]

    Signed-off-by: David Howells
    cc: Jarkko Sakkinen
    cc: Paul Moore
    cc: Stephen Smalley
    cc: Casey Schaufler
    cc: keyrings@vger.kernel.org
    cc: selinux@vger.kernel.org

    David Howells
     
  • Add a key/keyring change notification facility whereby notifications about
    changes in key and keyring content and attributes can be received.

    Firstly, an event queue needs to be created:

    pipe2(fds, O_NOTIFICATION_PIPE);
    ioctl(fds[1], IOC_WATCH_QUEUE_SET_SIZE, 256);

    then a notification can be set up to report notifications via that queue:

    struct watch_notification_filter filter = {
    .nr_filters = 1,
    .filters = {
    [0] = {
    .type = WATCH_TYPE_KEY_NOTIFY,
    .subtype_filter[0] = UINT_MAX,
    },
    },
    };
    ioctl(fds[1], IOC_WATCH_QUEUE_SET_FILTER, &filter);
    keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fds[1], 0x01);

    After that, records will be placed into the queue when events occur in
    which keys are changed in some way. Records are of the following format:

    struct key_notification {
    struct watch_notification watch;
    __u32 key_id;
    __u32 aux;
    } *n;

    Where:

    n->watch.type will be WATCH_TYPE_KEY_NOTIFY.

    n->watch.subtype will indicate the type of event, such as
    NOTIFY_KEY_REVOKED.

    n->watch.info & WATCH_INFO_LENGTH will indicate the length of the
    record.

    n->watch.info & WATCH_INFO_ID will be the second argument to
    keyctl_watch_key(), shifted.

    n->key will be the ID of the affected key.

    n->aux will hold subtype-dependent information, such as the key
    being linked into the keyring specified by n->key in the case of
    NOTIFY_KEY_LINKED.

    Note that it is permissible for event records to be of variable length -
    or, at least, the length may be dependent on the subtype. Note also that
    the queue can be shared between multiple notifications of various types.

    Signed-off-by: David Howells
    Reviewed-by: James Morris

    David Howells
     

08 May, 2020

1 commit

  • Instead of manually allocating a 'struct shash_desc' on the stack and
    calling crypto_shash_digest(), switch to using the new helper function
    crypto_shash_tfm_digest() which does this for us.

    Cc: keyrings@vger.kernel.org
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     

17 Apr, 2020

1 commit

  • If seq_file .next function does not change position index,
    read after some lseek can generate unexpected output:

    $ dd if=/proc/keys bs=1 # full usual output
    0f6bfdf5 I--Q--- 2 perm 3f010000 1000 1000 user 4af2f79ab8848d0a: 740
    1fb91b32 I--Q--- 3 perm 1f3f0000 1000 65534 keyring _uid.1000: 2
    27589480 I--Q--- 1 perm 0b0b0000 0 0 user invocation_id: 16
    2f33ab67 I--Q--- 152 perm 3f030000 0 0 keyring _ses: 2
    33f1d8fa I--Q--- 4 perm 3f030000 1000 1000 keyring _ses: 1
    3d427fda I--Q--- 2 perm 3f010000 1000 1000 user 69ec44aec7678e5a: 740
    3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1
    521+0 records in
    521+0 records out
    521 bytes copied, 0,00123769 s, 421 kB/s

    But a read after lseek in middle of last line results in the partial
    last line and then a repeat of the final line:

    $ dd if=/proc/keys bs=500 skip=1
    dd: /proc/keys: cannot skip to specified offset
    g _uid_ses.1000: 1
    3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1
    0+1 records in
    0+1 records out
    97 bytes copied, 0,000135035 s, 718 kB/s

    and a read after lseek beyond end of file results in the last line being
    shown:

    $ dd if=/proc/keys bs=1000 skip=1 # read after lseek beyond end of file
    dd: /proc/keys: cannot skip to specified offset
    3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1
    0+1 records in
    0+1 records out
    76 bytes copied, 0,000119981 s, 633 kB/s

    See https://bugzilla.kernel.org/show_bug.cgi?id=206283

    Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code ...")
    Signed-off-by: Vasily Averin
    Signed-off-by: David Howells
    Reviewed-by: Jarkko Sakkinen
    Cc: stable@vger.kernel.org
    Signed-off-by: Linus Torvalds

    Vasily Averin
     

05 Apr, 2020

1 commit

  • Pull keyrings fixes from David Howells:
    "Here's a couple of patches that fix a circular dependency between
    holding key->sem and mm->mmap_sem when reading data from a key.

    One potential issue is that a filesystem looking to use a key inside,
    say, ->readpages() could deadlock if the key being read is the key
    that's required and the buffer the key is being read into is on a page
    that needs to be fetched.

    The case actually detected is a bit more involved - with a filesystem
    calling request_key() and locking the target keyring for write - which
    could be being read"

    * tag 'keys-fixes-20200329' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
    KEYS: Avoid false positive ENOMEM error on key read
    KEYS: Don't write out to userspace while holding key semaphore

    Linus Torvalds
     

29 Mar, 2020

2 commits

  • By allocating a kernel buffer with a user-supplied buffer length, it
    is possible that a false positive ENOMEM error may be returned because
    the user-supplied length is just too large even if the system do have
    enough memory to hold the actual key data.

    Moreover, if the buffer length is larger than the maximum amount of
    memory that can be returned by kmalloc() (2^(MAX_ORDER-1) number of
    pages), a warning message will also be printed.

    To reduce this possibility, we set a threshold (PAGE_SIZE) over which we
    do check the actual key length first before allocating a buffer of the
    right size to hold it. The threshold is arbitrary, it is just used to
    trigger a buffer length check. It does not limit the actual key length
    as long as there is enough memory to satisfy the memory request.

    To further avoid large buffer allocation failure due to page
    fragmentation, kvmalloc() is used to allocate the buffer so that vmapped
    pages can be used when there is not a large enough contiguous set of
    pages available for allocation.

    In the extremely unlikely scenario that the key keeps on being changed
    and made longer (still
    Signed-off-by: David Howells

    Waiman Long
     
  • A lockdep circular locking dependency report was seen when running a
    keyutils test:

    [12537.027242] ======================================================
    [12537.059309] WARNING: possible circular locking dependency detected
    [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - -
    [12537.125253] ------------------------------------------------------
    [12537.153189] keyctl/25598 is trying to acquire lock:
    [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0
    [12537.208365]
    [12537.208365] but task is already holding lock:
    [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
    [12537.270476]
    [12537.270476] which lock already depends on the new lock.
    [12537.270476]
    [12537.307209]
    [12537.307209] the existing dependency chain (in reverse order) is:
    [12537.340754]
    [12537.340754] -> #3 (&type->lock_class){++++}:
    [12537.367434] down_write+0x4d/0x110
    [12537.385202] __key_link_begin+0x87/0x280
    [12537.405232] request_key_and_link+0x483/0xf70
    [12537.427221] request_key+0x3c/0x80
    [12537.444839] dns_query+0x1db/0x5a5 [dns_resolver]
    [12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
    [12537.496731] cifs_reconnect+0xe04/0x2500 [cifs]
    [12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs]
    [12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs]
    [12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
    [12537.601045] kthread+0x30c/0x3d0
    [12537.617906] ret_from_fork+0x3a/0x50
    [12537.636225]
    [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}:
    [12537.664525] __mutex_lock+0x105/0x11f0
    [12537.683734] request_key_and_link+0x35a/0xf70
    [12537.705640] request_key+0x3c/0x80
    [12537.723304] dns_query+0x1db/0x5a5 [dns_resolver]
    [12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
    [12537.775607] cifs_reconnect+0xe04/0x2500 [cifs]
    [12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs]
    [12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs]
    [12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
    [12537.873477] kthread+0x30c/0x3d0
    [12537.890281] ret_from_fork+0x3a/0x50
    [12537.908649]
    [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}:
    [12537.935225] __mutex_lock+0x105/0x11f0
    [12537.954450] cifs_call_async+0x102/0x7f0 [cifs]
    [12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs]
    [12538.000659] cifs_readpages+0x120a/0x1e50 [cifs]
    [12538.023920] read_pages+0xf5/0x560
    [12538.041583] __do_page_cache_readahead+0x41d/0x4b0
    [12538.067047] ondemand_readahead+0x44c/0xc10
    [12538.092069] filemap_fault+0xec1/0x1830
    [12538.111637] __do_fault+0x82/0x260
    [12538.129216] do_fault+0x419/0xfb0
    [12538.146390] __handle_mm_fault+0x862/0xdf0
    [12538.167408] handle_mm_fault+0x154/0x550
    [12538.187401] __do_page_fault+0x42f/0xa60
    [12538.207395] do_page_fault+0x38/0x5e0
    [12538.225777] page_fault+0x1e/0x30
    [12538.243010]
    [12538.243010] -> #0 (&mm->mmap_sem){++++}:
    [12538.267875] lock_acquire+0x14c/0x420
    [12538.286848] __might_fault+0x119/0x1b0
    [12538.306006] keyring_read_iterator+0x7e/0x170
    [12538.327936] assoc_array_subtree_iterate+0x97/0x280
    [12538.352154] keyring_read+0xe9/0x110
    [12538.370558] keyctl_read_key+0x1b9/0x220
    [12538.391470] do_syscall_64+0xa5/0x4b0
    [12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf
    [12538.435535]
    [12538.435535] other info that might help us debug this:
    [12538.435535]
    [12538.472829] Chain exists of:
    [12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class
    [12538.472829]
    [12538.524820] Possible unsafe locking scenario:
    [12538.524820]
    [12538.551431] CPU0 CPU1
    [12538.572654] ---- ----
    [12538.595865] lock(&type->lock_class);
    [12538.613737] lock(root_key_user.cons_lock);
    [12538.644234] lock(&type->lock_class);
    [12538.672410] lock(&mm->mmap_sem);
    [12538.687758]
    [12538.687758] *** DEADLOCK ***
    [12538.687758]
    [12538.714455] 1 lock held by keyctl/25598:
    [12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
    [12538.770573]
    [12538.770573] stack backtrace:
    [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G
    [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015
    [12538.881963] Call Trace:
    [12538.892897] dump_stack+0x9a/0xf0
    [12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279
    [12538.932891] ? save_trace+0xd6/0x250
    [12538.948979] check_prev_add.constprop.32+0xc36/0x14f0
    [12538.971643] ? keyring_compare_object+0x104/0x190
    [12538.992738] ? check_usage+0x550/0x550
    [12539.009845] ? sched_clock+0x5/0x10
    [12539.025484] ? sched_clock_cpu+0x18/0x1e0
    [12539.043555] __lock_acquire+0x1f12/0x38d0
    [12539.061551] ? trace_hardirqs_on+0x10/0x10
    [12539.080554] lock_acquire+0x14c/0x420
    [12539.100330] ? __might_fault+0xc4/0x1b0
    [12539.119079] __might_fault+0x119/0x1b0
    [12539.135869] ? __might_fault+0xc4/0x1b0
    [12539.153234] keyring_read_iterator+0x7e/0x170
    [12539.172787] ? keyring_read+0x110/0x110
    [12539.190059] assoc_array_subtree_iterate+0x97/0x280
    [12539.211526] keyring_read+0xe9/0x110
    [12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0
    [12539.249076] keyctl_read_key+0x1b9/0x220
    [12539.266660] do_syscall_64+0xa5/0x4b0
    [12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf

    One way to prevent this deadlock scenario from happening is to not
    allow writing to userspace while holding the key semaphore. Instead,
    an internal buffer is allocated for getting the keys out from the
    read method first before copying them out to userspace without holding
    the lock.

    That requires taking out the __user modifier from all the relevant
    read methods as well as additional changes to not use any userspace
    write helpers. That is,

    1) The put_user() call is replaced by a direct copy.
    2) The copy_to_user() call is replaced by memcpy().
    3) All the fault handling code is removed.

    Compiling on a x86-64 system, the size of the rxrpc_read() function is
    reduced from 3795 bytes to 2384 bytes with this patch.

    Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2")
    Reviewed-by: Jarkko Sakkinen
    Signed-off-by: Waiman Long
    Signed-off-by: David Howells

    Waiman Long
     

16 Mar, 2020

1 commit

  • Currently, when we add a new user key, the calltrace as below:

    add_key()
    key_create_or_update()
    key_alloc()
    __key_instantiate_and_link
    generic_key_instantiate
    key_payload_reserve
    ......

    Since commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly"),
    we can reach max bytes/keys in key_alloc, but we forget to remove this
    limit when we reserver space for payload in key_payload_reserve. So we
    can only reach max keys but not max bytes when having delta between plen
    and type->def_datalen. Remove this limit when instantiating the key, so we
    can keep consistent with key_alloc.

    Also, fix the similar problem in keyctl_chown_key().

    Fixes: 0b77f5bfb45c ("keys: make the keyring quotas controllable through /proc/sys")
    Fixes: a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
    Cc: stable@vger.kernel.org # 5.0.x
    Cc: Eric Biggers
    Signed-off-by: Yang Xu
    Reviewed-by: Jarkko Sakkinen
    Reviewed-by: Eric Biggers
    Signed-off-by: Jarkko Sakkinen

    Yang Xu
     

29 Jan, 2020

1 commit

  • Pull IMA updates from Mimi Zohar:
    "Two new features - measuring certificates and querying IMA for a file
    hash - and three bug fixes:

    - Measuring certificates is like the rest of IMA, based on policy,
    but requires loading a custom policy. Certificates loaded onto a
    keyring, for example during early boot, before a custom policy has
    been loaded, are queued and only processed after loading the custom
    policy.

    - IMA calculates and caches files hashes. Other kernel subsystems,
    and possibly kernel modules, are interested in accessing these
    cached file hashes.

    The bug fixes prevent classifying a file short read (e.g. shutdown) as
    an invalid file signature, add a missing blank when displaying the
    securityfs policy rules containing LSM labels, and, lastly, fix the
    handling of the IMA policy information for unknown LSM labels"

    * 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity:
    IMA: Defined delayed workqueue to free the queued keys
    IMA: Call workqueue functions to measure queued keys
    IMA: Define workqueue for early boot key measurements
    IMA: pre-allocate buffer to hold keyrings string
    ima: ima/lsm policy rule loading logic bug fixes
    ima: add the ability to query the cached hash of a given file
    ima: Add a space after printing LSM rules for readability
    IMA: fix measuring asymmetric keys Kconfig
    IMA: Read keyrings= option from the IMA policy
    IMA: Add support to limit measuring keys
    KEYS: Call the IMA hook to measure keys
    IMA: Define an IMA hook to measure keys
    IMA: Add KEY_CHECK func to measure keys
    IMA: Check IMA policy flag
    ima: avoid appraise error for hash calc interrupt

    Linus Torvalds
     

17 Dec, 2019

1 commit

  • The original code, before it was moved into security/keys/trusted-keys
    had a flush after the blob unseal. Without that flush, the volatile
    handles increase in the TPM until it becomes unusable and the system
    either has to be rebooted or the TPM volatile area manually flushed.
    Fix by adding back the lost flush, which we now have to export because
    of the relocation of the trusted key code may cause the consumer to be
    modular.

    Signed-off-by: James Bottomley
    Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")
    Reviewed-by: Jerry Snitselaar
    Reviewed-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    James Bottomley
     

13 Dec, 2019

1 commit

  • KEYS_COMPAT now always takes the value of COMPAT && KEYS. But the
    security/keys/ directory is only compiled if KEYS is enabled, so in
    practice KEYS_COMPAT is the same as COMPAT. Therefore, remove the
    unnecessary KEYS_COMPAT and just use COMPAT directly.

    (Also remove an outdated comment from compat.c.)

    Reviewed-by: James Morris
    Signed-off-by: Eric Biggers
    Reviewed-by: Jarkko Sakkinen
    Tested-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Eric Biggers
     

12 Dec, 2019

1 commit

  • Call the IMA hook from key_create_or_update() function to measure
    the payload when a new key is created or an existing key is updated.

    This patch adds the call to the IMA hook from key_create_or_update()
    function to measure the key on key create or update.

    Signed-off-by: Lakshmi Ramasubramanian
    Cc: David Howells
    Cc: Jarkko Sakkinen
    Signed-off-by: Mimi Zohar

    Lakshmi Ramasubramanian
     

13 Nov, 2019

5 commits

  • Fixes gcc '-Wunused-but-set-variable' warning:

    security/keys/trusted-keys/trusted_tpm1.c: In function tpm_unseal:
    security/keys/trusted-keys/trusted_tpm1.c:588:11: warning: variable keyhndl set but not used [-Wunused-but-set-variable]

    Fixes: 00aa975bd031 ("KEYS: trusted: Create trusted keys subsystem")
    Reported-by: Hulk Robot
    Signed-off-by: zhengbin
    Reviewed-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    zhengbin
     
  • Move TPM2 trusted keys code to trusted keys subsystem. The reason
    being it's better to consolidate all the trusted keys code to a single
    location so that it can be maintained sanely.

    Also, utilize existing tpm_send() exported API which wraps the internal
    tpm_transmit_cmd() API.

    Suggested-by: Jarkko Sakkinen
    Signed-off-by: Sumit Garg
    Reviewed-by: Jarkko Sakkinen
    Tested-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Sumit Garg
     
  • Move existing code to trusted keys subsystem. Also, rename files with
    "tpm" as suffix which provides the underlying implementation.

    Suggested-by: Jarkko Sakkinen
    Signed-off-by: Sumit Garg
    Reviewed-by: Jarkko Sakkinen
    Tested-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Sumit Garg
     
  • Switch to utilize common heap based tpm_buf code for TPM based trusted
    and asymmetric keys rather than using stack based tpm1_buf code. Also,
    remove tpm1_buf code.

    Suggested-by: Jarkko Sakkinen
    Signed-off-by: Sumit Garg
    Reviewed-by: Jerry Snitselaar
    Reviewed-by: Jarkko Sakkinen
    Tested-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Sumit Garg
     
  • Move tpm_buf code to common include/linux/tpm.h header so that it can
    be reused via other subsystems like trusted keys etc.

    Also rename trusted keys and asymmetric keys usage of TPM 1.x buffer
    implementation to tpm1_buf to avoid any compilation errors.

    Suggested-by: Jarkko Sakkinen
    Signed-off-by: Sumit Garg
    Reviewed-by: Jerry Snitselaar
    Reviewed-by: Jarkko Sakkinen
    Tested-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Sumit Garg
     

25 Sep, 2019

1 commit

  • Commit 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to
    tpm_pcr_extend()") modifies tpm_pcr_extend() to accept a digest for each
    PCR bank. After modification, tpm_pcr_extend() expects that digests are
    passed in the same order as the algorithms set in chip->allocated_banks.

    This patch fixes two issues introduced in the last iterations of the patch
    set: missing initialization of the TPM algorithm ID in the tpm_digest
    structures passed to tpm_pcr_extend() by the trusted key module, and
    unreleased locks in the TPM driver due to returning from tpm_pcr_extend()
    without calling tpm_put_ops().

    Cc: stable@vger.kernel.org
    Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
    Signed-off-by: Roberto Sassu
    Suggested-by: Jarkko Sakkinen
    Reviewed-by: Jerry Snitselaar
    Reviewed-by: Jarkko Sakkinen
    Signed-off-by: Jarkko Sakkinen

    Roberto Sassu
     

06 Sep, 2019

1 commit

  • If a request_key authentication token key gets revoked, there's a window in
    which request_key_auth_describe() can see it with a NULL payload - but it
    makes no check for this and something like the following oops may occur:

    BUG: Kernel NULL pointer dereference at 0x00000038
    Faulting instruction address: 0xc0000000004ddf30
    Oops: Kernel access of bad area, sig: 11 [#1]
    ...
    NIP [...] request_key_auth_describe+0x90/0xd0
    LR [...] request_key_auth_describe+0x54/0xd0
    Call Trace:
    [...] request_key_auth_describe+0x54/0xd0 (unreliable)
    [...] proc_keys_show+0x308/0x4c0
    [...] seq_read+0x3d0/0x540
    [...] proc_reg_read+0x90/0x110
    [...] __vfs_read+0x3c/0x70
    [...] vfs_read+0xb4/0x1b0
    [...] ksys_read+0x7c/0x130
    [...] system_call+0x5c/0x70

    Fix this by checking for a NULL pointer when describing such a key.

    Also make the read routine check for a NULL pointer to be on the safe side.

    [DH: Modified to not take already-held rcu lock and modified to also check
    in the read routine]

    Fixes: 04c567d9313e ("[PATCH] Keys: Fix race between two instantiators of a key")
    Reported-by: Sachin Sant
    Signed-off-by: Hillf Danton
    Signed-off-by: David Howells
    Tested-by: Sachin Sant
    Signed-off-by: Linus Torvalds

    Hillf Danton
     

31 Aug, 2019

1 commit