1. 27 Aug, 2019 2 commits
  2. 19 Aug, 2019 1 commit
    • Eric W. Biederman's avatar
      signal: Allow cifs and drbd to receive their terminating signals · 33da8e7c
      Eric W. Biederman authored
      
      
      My recent to change to only use force_sig for a synchronous events
      wound up breaking signal reception cifs and drbd.  I had overlooked
      the fact that by default kthreads start out with all signals set to
      SIG_IGN.  So a change I thought was safe turned out to have made it
      impossible for those kernel thread to catch their signals.
      
      Reverting the work on force_sig is a bad idea because what the code
      was doing was very much a misuse of force_sig.  As the way force_sig
      ultimately allowed the signal to happen was to change the signal
      handler to SIG_DFL.  Which after the first signal will allow userspace
      to send signals to these kernel threads.  At least for
      wake_ack_receiver in drbd that does not appear actively wrong.
      
      So correct this problem by adding allow_kernel_signal that will allow
      signals whose siginfo reports they were sent by the kernel through,
      but will not allow userspace generated signals, and update cifs and
      drbd to call allow_kernel_signal in an appropriate place so that their
      thread can receive this signal.
      
      Fixing things this way ensures that userspace won't be able to send
      signals and cause problems, that it is clear which signals the
      threads are expecting to receive, and it guarantees that nothing
      else in the system will be affected.
      
      This change was partly inspired by similar cifs and drbd patches that
      added allow_signal.
      Reported-by: default avatarronnie sahlberg <ronniesahlberg@gmail.com>
      Reported-by: default avatarChristoph Böhmwalder <christoph.boehmwalder@linbit.com>
      Tested-by: default avatarChristoph Böhmwalder <christoph.boehmwalder@linbit.com>
      Cc: Steve French <smfrench@gmail.com>
      Cc: Philipp Reisner <philipp.reisner@linbit.com>
      Cc: David Laight <David.Laight@ACULAB.COM>
      Fixes: 247bc947 ("cifs: fix rmmod regression in cifs.ko caused by force_sig changes")
      Fixes: 72abe3bc ("signal/cifs: Fix cifs_put_tcp_session to call send_sig instead of force_sig")
      Fixes: fee10990 ("signal/drbd: Use send_sig not force_sig")
      Fixes: 3cf5d076
      
       ("signal: Remove task parameter from force_sig")
      Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
      33da8e7c
  3. 05 Aug, 2019 1 commit
  4. 13 Jul, 2019 1 commit
  5. 11 Jul, 2019 1 commit
  6. 08 Jul, 2019 6 commits
  7. 27 Jun, 2019 1 commit
    • David Howells's avatar
      keys: Replace uid/gid/perm permissions checking with an ACL · 2e12256b
      David Howells authored
      
      
      Replace the uid/gid/perm permissions checking on a key with an ACL to allow
      the SETATTR and SEARCH permissions to be split.  This will also allow a
      greater range of subjects to represented.
      
      ============
      WHY DO THIS?
      ============
      
      The problem is that SETATTR and SEARCH cover a slew of actions, not all of
      which should be grouped together.
      
      For SETATTR, this includes actions that are about controlling access to a
      key:
      
       (1) Changing a key's ownership.
      
       (2) Changing a key's security information.
      
       (3) Setting a keyring's restriction.
      
      And actions that are about managing a key's lifetime:
      
       (4) Setting an expiry time.
      
       (5) Revoking a key.
      
      and (proposed) managing a key as part of a cache:
      
       (6) Invalidating a key.
      
      Managing a key's lifetime doesn't really have anything to do with
      controlling access to that key.
      
      Expiry time is awkward since it's more about the lifetime of the content
      and so, in some ways goes better with WRITE permission.  It can, however,
      be set unconditionally by a process with an appropriate authorisation token
      for instantiating a key, and can also be set by the key type driver when a
      key is instantiated, so lumping it with the access-controlling actions is
      probably okay.
      
      As for SEARCH permission, that currently covers:
      
       (1) Finding keys in a keyring tree during a search.
      
       (2) Permitting keyrings to be joined.
      
       (3) Invalidation.
      
      But these don't really belong together either, since these actions really
      need to be controlled separately.
      
      Finally, there are number of special cases to do with granting the
      administrator special rights to invalidate or clear keys that I would like
      to handle with the ACL rather than key flags and special checks.
      
      
      ===============
      WHAT IS CHANGED
      ===============
      
      The SETATTR permission is split to create two new permissions:
      
       (1) SET_SECURITY - which allows the key's owner, group and ACL to be
           changed and a restriction to be placed on a keyring.
      
       (2) REVOKE - which allows a key to be revoked.
      
      The SEARCH permission is split to create:
      
       (1) SEARCH - which allows a keyring to be search and a key to be found.
      
       (2) JOIN - which allows a keyring to be joined as a session keyring.
      
       (3) INVAL - which allows a key to be invalidated.
      
      The WRITE permission is also split to create:
      
       (1) WRITE - which allows a key's content to be altered and links to be
           added, removed and replaced in a keyring.
      
       (2) CLEAR - which allows a keyring to be cleared completely.  This is
           split out to make it possible to give just this to an administrator.
      
       (3) REVOKE - see above.
      
      
      Keys acquire ACLs which consist of a series of ACEs, and all that apply are
      unioned together.  An ACE specifies a subject, such as:
      
       (*) Possessor - permitted to anyone who 'possesses' a key
       (*) Owner - permitted to the key owner
       (*) Group - permitted to the key group
       (*) Everyone - permitted to everyone
      
      Note that 'Other' has been replaced with 'Everyone' on the assumption that
      you wouldn't grant a permit to 'Other' that you wouldn't also grant to
      everyone else.
      
      Further subjects may be made available by later patches.
      
      The ACE also specifies a permissions mask.  The set of permissions is now:
      
      	VIEW		Can view the key metadata
      	READ		Can read the key content
      	WRITE		Can update/modify the key content
      	SEARCH		Can find the key by searching/requesting
      	LINK		Can make a link to the key
      	SET_SECURITY	Can change owner, ACL, expiry
      	INVAL		Can invalidate
      	REVOKE		Can revoke
      	JOIN		Can join this keyring
      	CLEAR		Can clear this keyring
      
      
      The KEYCTL_SETPERM function is then deprecated.
      
      The KEYCTL_SET_TIMEOUT function then is permitted if SET_SECURITY is set,
      or if the caller has a valid instantiation auth token.
      
      The KEYCTL_INVALIDATE function then requires INVAL.
      
      The KEYCTL_REVOKE function then requires REVOKE.
      
      The KEYCTL_JOIN_SESSION_KEYRING function then requires JOIN to join an
      existing keyring.
      
      The JOIN permission is enabled by default for session keyrings and manually
      created keyrings only.
      
      
      ======================
      BACKWARD COMPATIBILITY
      ======================
      
      To maintain backward compatibility, KEYCTL_SETPERM will translate the
      permissions mask it is given into a new ACL for a key - unless
      KEYCTL_SET_ACL has been called on that key, in which case an error will be
      returned.
      
      It will convert possessor, owner, group and other permissions into separate
      ACEs, if each portion of the mask is non-zero.
      
      SETATTR permission turns on all of INVAL, REVOKE and SET_SECURITY.  WRITE
      permission turns on WRITE, REVOKE and, if a keyring, CLEAR.  JOIN is turned
      on if a keyring is being altered.
      
      The KEYCTL_DESCRIBE function translates the ACL back into a permissions
      mask to return depending on possessor, owner, group and everyone ACEs.
      
      It will make the following mappings:
      
       (1) INVAL, JOIN -> SEARCH
      
       (2) SET_SECURITY -> SETATTR
      
       (3) REVOKE -> WRITE if SETATTR isn't already set
      
       (4) CLEAR -> WRITE
      
      Note that the value subsequently returned by KEYCTL_DESCRIBE may not match
      the value set with KEYCTL_SETATTR.
      
      
      =======
      TESTING
      =======
      
      This passes the keyutils testsuite for all but a couple of tests:
      
       (1) tests/keyctl/dh_compute/badargs: The first wrong-key-type test now
           returns EOPNOTSUPP rather than ENOKEY as READ permission isn't removed
           if the type doesn't have ->read().  You still can't actually read the
           key.
      
       (2) tests/keyctl/permitting/valid: The view-other-permissions test doesn't
           work as Other has been replaced with Everyone in the ACL.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      2e12256b
  8. 17 Jun, 2019 1 commit
  9. 27 May, 2019 1 commit
    • Eric W. Biederman's avatar
      signal/cifs: Fix cifs_put_tcp_session to call send_sig instead of force_sig · 72abe3bc
      Eric W. Biederman authored
      The locking in force_sig_info is not prepared to deal with a task that
      exits or execs (as sighand may change).  The is not a locking problem
      in force_sig as force_sig is only built to handle synchronous
      exceptions.
      
      Further the function force_sig_info changes the signal state if the
      signal is ignored, or blocked or if SIGNAL_UNKILLABLE will prevent the
      delivery of the signal.  The signal SIGKILL can not be ignored and can
      not be blocked and SIGNAL_UNKILLABLE won't prevent it from being
      delivered.
      
      So using force_sig rather than send_sig for SIGKILL is confusing
      and pointless.
      
      Because it won't impact the sending of the signal and and because
      using force_sig is wrong, replace force_sig with send_sig.
      
      Cc: Namjae Jeon <namjae.jeon@samsung.com>
      Cc: Jeff Layton <jlayton@primarydata.com>
      Cc: Steve French <smfrench@gmail.com>
      Fixes: a5c3e1c7 ("Revert "cifs: No need to send SIGKILL to demux_thread during umount"")
      Fixes: e7ddee90
      
       ("cifs: disable sharing session and tcon and add new TCP sharing code")
      Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
      72abe3bc
  10. 16 May, 2019 1 commit
  11. 14 May, 2019 1 commit
  12. 08 May, 2019 3 commits
  13. 01 Apr, 2019 1 commit
    • Steve French's avatar
      SMB3: Allow persistent handle timeout to be configurable on mount · ca567eb2
      Steve French authored
      
      
      Reconnecting after server or network failure can be improved
      (to maintain availability and protect data integrity) by allowing
      the client to choose the default persistent (or resilient)
      handle timeout in some use cases.  Today we default to 0 which lets
      the server pick the default timeout (usually 120 seconds) but this
      can be problematic for some workloads.  Add the new mount parameter
      to cifs.ko for SMB3 mounts "handletimeout" which enables the user
      to override the default handle timeout for persistent (mount
      option "persistenthandles") or resilient handles (mount option
      "resilienthandles").  Maximum allowed is 16 minutes (960000 ms).
      Units for the timeout are expressed in milliseconds. See
      section 2.2.14.2.12 and 2.2.31.3 of the MS-SMB2 protocol
      specification for more information.
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      Reviewed-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      Reviewed-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      CC: Stable <stable@vger.kernel.org>
      ca567eb2
  14. 15 Mar, 2019 1 commit
  15. 06 Mar, 2019 2 commits
  16. 05 Mar, 2019 5 commits
    • Pavel Shilovsky's avatar
      CIFS: Count SMB3 credits for malformed pending responses · 66265f13
      Pavel Shilovsky authored
      
      
      Even if a response is malformed, we should count credits
      granted by the server to avoid miscalculations and unnecessary
      reconnects due to client or server bugs. If the response has
      been received partially, the session will be reconnected anyway
      on the next iteration of the demultiplex thread, so counting
      credits for such cases shouldn't break things.
      Signed-off-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      66265f13
    • Steve French's avatar
      smb3: make default i/o size for smb3 mounts larger · e8506d25
      Steve French authored
      
      
      We negotiate rsize mounts (and it can be overridden by user) to
      typically 4MB, so using larger default I/O sizes from userspace
      (changing to 1MB default i/o size returned by stat) the
      performance is much better (and not just for long latency
      network connections) in most use cases for SMB3 than the default I/O
      size (which ends up being 128K for cp and can be even smaller for cp).
      This can be 4x slower or worse depending on network latency.
      
      By changing inode->blocksize from 32K (which was perhaps ok
      for very old SMB1/CIFS) to a larger value, 1MB (but still less than
      max size negotiated with the server which is 4MB, in order to minimize
      risk) it significantly increases performance for the
      noncached case, and slightly increases it for the cached case.
      This can be changed by the user on mount (specifying bsize=
      values from 16K to 16MB) to tune better for performance
      for applications that depend on blocksize.
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      Reviewed-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      CC: Stable <stable@vger.kernel.org>
      e8506d25
    • Ronnie Sahlberg's avatar
      cifs: add credits from unmatched responses/messages · eca00452
      Ronnie Sahlberg authored
      
      
      We should add any credits granted to us from unmatched server responses.
      Signed-off-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      Reviewed-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      eca00452
    • Ronnie Sahlberg's avatar
      cifs: replace snprintf with scnprintf · 74ea5f98
      Ronnie Sahlberg authored
      
      
      a trivial patch that replaces all use of snprintf with scnprintf.
      scnprintf() is generally seen as a safer function to use than
      snprintf for many use cases.
      
      In our case, there is no actual difference between the two since we never
      look at the return value. Thus we did not have any of the bugs that
      scnprintf protects against and the patch does nothing.
      
      However, for people reading our code it will be a receipt that we
      have done our due dilligence and checked our code for this type of bugs.
      
      See the presentation "Making C Less Dangerous In The Linux Kernel"
      at this years LCA
      Signed-off-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      74ea5f98
    • Yao Liu's avatar
      cifs: Fix NULL pointer dereference of devname · 68e2672f
      Yao Liu authored
      
      
      There is a NULL pointer dereference of devname in strspn()
      
      The oops looks something like:
      
        CIFS: Attempting to mount (null)
        BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
        ...
        RIP: 0010:strspn+0x0/0x50
        ...
        Call Trace:
         ? cifs_parse_mount_options+0x222/0x1710 [cifs]
         ? cifs_get_volume_info+0x2f/0x80 [cifs]
         cifs_setup_volume_info+0x20/0x190 [cifs]
         cifs_get_volume_info+0x50/0x80 [cifs]
         cifs_smb3_do_mount+0x59/0x630 [cifs]
         ? ida_alloc_range+0x34b/0x3d0
         cifs_do_mount+0x11/0x20 [cifs]
         mount_fs+0x52/0x170
         vfs_kern_mount+0x6b/0x170
         do_mount+0x216/0xdc0
         ksys_mount+0x83/0xd0
         __x64_sys_mount+0x25/0x30
         do_syscall_64+0x65/0x220
         entry_SYSCALL_64_after_hwframe+0x49/0xbe
      
      Fix this by adding a NULL check on devname in cifs_parse_devname()
      Signed-off-by: default avatarYao Liu <yotta.liu@ucloud.cn>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      68e2672f
  17. 24 Jan, 2019 1 commit
  18. 10 Jan, 2019 1 commit
  19. 03 Jan, 2019 1 commit
  20. 28 Dec, 2018 8 commits