13 Oct, 2017

1 commit

  • digsig_verify() requests a user key, then accesses its payload.
    However, a revoked key has a NULL payload, and we failed to check for
    this. request_key() *does* skip revoked keys, but there is still a
    window where the key can be revoked before we acquire its semaphore.

    Fix it by checking for a NULL payload, treating it like a key which was
    already revoked at the time it was requested.

    Fixes: 051dbb918c7f ("crypto: digital signature verification support")
    Reviewed-by: James Morris
    Cc: [v3.3+]
    Cc: Dmitry Kasatkin
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     

02 Mar, 2017

1 commit

  • rcu_dereference_key() and user_key_payload() are currently being used in
    two different, incompatible ways:

    (1) As a wrapper to rcu_dereference() - when only the RCU read lock used
    to protect the key.

    (2) As a wrapper to rcu_dereference_protected() - when the key semaphor is
    used to protect the key and the may be being modified.

    Fix this by splitting both of the key wrappers to produce:

    (1) RCU accessors for keys when caller has the key semaphore locked:

    dereference_key_locked()
    user_key_payload_locked()

    (2) RCU accessors for keys when caller holds the RCU read lock:

    dereference_key_rcu()
    user_key_payload_rcu()

    This should fix following warning in the NFS idmapper

    ===============================
    [ INFO: suspicious RCU usage. ]
    4.10.0 #1 Tainted: G W
    -------------------------------
    ./include/keys/user-type.h:53 suspicious rcu_dereference_protected() usage!
    other info that might help us debug this:
    rcu_scheduler_active = 2, debug_locks = 0
    1 lock held by mount.nfs/5987:
    #0: (rcu_read_lock){......}, at: [] nfs_idmap_get_key+0x15c/0x420 [nfsv4]
    stack backtrace:
    CPU: 1 PID: 5987 Comm: mount.nfs Tainted: G W 4.10.0 #1
    Call Trace:
    dump_stack+0xe8/0x154 (unreliable)
    lockdep_rcu_suspicious+0x140/0x190
    nfs_idmap_get_key+0x380/0x420 [nfsv4]
    nfs_map_name_to_uid+0x2a0/0x3b0 [nfsv4]
    decode_getfattr_attrs+0xfac/0x16b0 [nfsv4]
    decode_getfattr_generic.constprop.106+0xbc/0x150 [nfsv4]
    nfs4_xdr_dec_lookup_root+0xac/0xb0 [nfsv4]
    rpcauth_unwrap_resp+0xe8/0x140 [sunrpc]
    call_decode+0x29c/0x910 [sunrpc]
    __rpc_execute+0x140/0x8f0 [sunrpc]
    rpc_run_task+0x170/0x200 [sunrpc]
    nfs4_call_sync_sequence+0x68/0xa0 [nfsv4]
    _nfs4_lookup_root.isra.44+0xd0/0xf0 [nfsv4]
    nfs4_lookup_root+0xe0/0x350 [nfsv4]
    nfs4_lookup_root_sec+0x70/0xa0 [nfsv4]
    nfs4_find_root_sec+0xc4/0x100 [nfsv4]
    nfs4_proc_get_rootfh+0x5c/0xf0 [nfsv4]
    nfs4_get_rootfh+0x6c/0x190 [nfsv4]
    nfs4_server_common_setup+0xc4/0x260 [nfsv4]
    nfs4_create_server+0x278/0x3c0 [nfsv4]
    nfs4_remote_mount+0x50/0xb0 [nfsv4]
    mount_fs+0x74/0x210
    vfs_kern_mount+0x78/0x220
    nfs_do_root_mount+0xb0/0x140 [nfsv4]
    nfs4_try_mount+0x60/0x100 [nfsv4]
    nfs_fs_mount+0x5ec/0xda0 [nfs]
    mount_fs+0x74/0x210
    vfs_kern_mount+0x78/0x220
    do_mount+0x254/0xf70
    SyS_mount+0x94/0x100
    system_call+0x38/0xe0

    Reported-by: Jan Stancek
    Signed-off-by: David Howells
    Tested-by: Jan Stancek
    Signed-off-by: James Morris

    David Howells
     

31 May, 2016

2 commits

  • Currently, if digsig_verify_rsa() detects that the modulo's length is zero,
    i.e. mlen == 0, it returns -ENOMEM which doesn't really fit here.

    Make digsig_verify_rsa() return -EINVAL upon mlen == 0.

    Signed-off-by: Nicolai Stange
    Signed-off-by: Herbert Xu

    Nicolai Stange
     
  • mpi_read_from_buffer() reads a MPI from a buffer into a newly allocated
    MPI instance. It expects the buffer's leading two bytes to contain the
    number of bits, followed by the actual payload.

    On failure, it returns NULL and updates the in/out argument ret_nread
    somewhat inconsistently:
    - If the given buffer is too short to contain the leading two bytes
    encoding the number of bits or their value is unsupported, then
    ret_nread will be cleared.
    - If the allocation of the resulting MPI instance fails, ret_nread is left
    as is.

    The only user of mpi_read_from_buffer(), digsig_verify_rsa(), simply checks
    for a return value of NULL and returns -ENOMEM if that happens.

    While this is all of cosmetic nature only, there is another error condition
    which currently isn't detectable by the caller of mpi_read_from_buffer():
    if the given buffer is too small to hold the number of bits as encoded in
    its first two bytes, the return value will be non-NULL and *ret_nread > 0.

    In preparation of communicating this condition to the caller, let
    mpi_read_from_buffer() return error values by means of the ERR_PTR()
    mechanism.

    Make the sole caller of mpi_read_from_buffer(), digsig_verify_rsa(),
    check the return value for IS_ERR() rather than == NULL. If IS_ERR() is
    true, return the associated error value rather than the fixed -ENOMEM.

    Signed-off-by: Nicolai Stange
    Signed-off-by: Herbert Xu

    Nicolai Stange
     

21 Oct, 2015

1 commit

  • Merge the type-specific data with the payload data into one four-word chunk
    as it seems pointless to keep them separate.

    Use user_key_payload() for accessing the payloads of overloaded
    user-defined keys.

    Signed-off-by: David Howells
    cc: linux-cifs@vger.kernel.org
    cc: ecryptfs@vger.kernel.org
    cc: linux-ext4@vger.kernel.org
    cc: linux-f2fs-devel@lists.sourceforge.net
    cc: linux-nfs@vger.kernel.org
    cc: ceph-devel@vger.kernel.org
    cc: linux-ima-devel@lists.sourceforge.net

    David Howells