Skip to content
Snippets Groups Projects
  1. Jan 14, 2025
  2. Sep 17, 2024
    • Ivan Orlov's avatar
      9P FS: Fix wild-memory-access write in v9fs_get_acl · d8e2b494
      Ivan Orlov authored
      
      [ Upstream commit 707823e7 ]
      
      KASAN reported the following issue:
      [   36.825817][ T5923] BUG: KASAN: wild-memory-access in v9fs_get_acl+0x1a4/0x390
      [   36.827479][ T5923] Write of size 4 at addr 9fffeb37f97f1c00 by task syz-executor798/5923
      [   36.829303][ T5923]
      [   36.829846][ T5923] CPU: 0 PID: 5923 Comm: syz-executor798 Not tainted 6.2.0-syzkaller-18302-g596b6b709632 #0
      [   36.832110][ T5923] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023
      [   36.834464][ T5923] Call trace:
      [   36.835196][ T5923]  dump_backtrace+0x1c8/0x1f4
      [   36.836229][ T5923]  show_stack+0x2c/0x3c
      [   36.837100][ T5923]  dump_stack_lvl+0xd0/0x124
      [   36.838103][ T5923]  print_report+0xe4/0x4c0
      [   36.839068][ T5923]  kasan_report+0xd4/0x130
      [   36.840052][ T5923]  kasan_check_range+0x264/0x2a4
      [   36.841199][ T5923]  __kasan_check_write+0x2c/0x3c
      [   36.842216][ T5923]  v9fs_get_acl+0x1a4/0x390
      [   36.843232][ T5923]  v9fs_mount+0x77c/0xa5c
      [   36.844163][ T5923]  legacy_get_tree+0xd4/0x16c
      [   36.845173][ T5923]  vfs_get_tree+0x90/0x274
      [   36.846137][ T5923]  do_new_mount+0x25c/0x8c8
      [   36.847066][ T5923]  path_mount+0x590/0xe58
      [   36.848147][ T5923]  __arm64_sys_mount+0x45c/0x594
      [   36.849273][ T5923]  invoke_syscall+0x98/0x2c0
      [   36.850421][ T5923]  el0_svc_common+0x138/0x258
      [   36.851397][ T5923]  do_el0_svc+0x64/0x198
      [   36.852398][ T5923]  el0_svc+0x58/0x168
      [   36.853224][ T5923]  el0t_64_sync_handler+0x84/0xf0
      [   36.854293][ T5923]  el0t_64_sync+0x190/0x194
      
      Calling '__v9fs_get_acl' method in 'v9fs_get_acl' creates the
      following chain of function calls:
      
      __v9fs_get_acl
      	v9fs_fid_get_acl
      		v9fs_fid_xattr_get
      			p9_client_xattrwalk
      
      Function p9_client_xattrwalk accepts a pointer to u64-typed
      variable attr_size and puts some u64 value into it. However,
      after the executing the p9_client_xattrwalk, in some circumstances
      we assign the value of u64-typed variable 'attr_size' to the
      variable 'retval', which we will return. However, the type of
      'retval' is ssize_t, and if the value of attr_size is larger
      than SSIZE_MAX, we will face the signed type overflow. If the
      overflow occurs, the result of v9fs_fid_xattr_get may be
      negative, but not classified as an error. When we try to allocate
      an acl with 'broken' size we receive an error, but don't process
      it. When we try to free this acl, we face the 'wild-memory-access'
      error (because it wasn't allocated).
      
      This patch will add new condition to the 'v9fs_fid_xattr_get'
      function, so it will return an EOVERFLOW error if the 'attr_size'
      is larger than SSIZE_MAX.
      
      In this version of the patch I simplified the condition.
      
      In previous (v2) version of the patch I removed explicit type conversion
      and added separate condition to check the possible overflow and return
      an error (in v1 version I've just modified the existing condition).
      
      Tested via syzkaller.
      
      Suggested-by: default avatarChristian Schoenebeck <linux_oss@crudebyte.com>
      Reported-by: default avatar <syzbot+cb1d16facb3cc90de5fb@syzkaller.appspotmail.com>
      Link: https://syzkaller.appspot.com/bug?id=fbbef66d9e4d096242f3617de5d14d12705b4659
      
      
      Signed-off-by: default avatarIvan Orlov <ivan.orlov0322@gmail.com>
      Reviewed-by: default avatarChristian Schoenebeck <linux_oss@crudebyte.com>
      Signed-off-by: default avatarEric Van Hensbergen <ericvh@kernel.org>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      d8e2b494
  3. Jul 11, 2024
    • Dominique Martinet's avatar
      9p: add missing locking around taking dentry fid list · d0b4e409
      Dominique Martinet authored and Frieder Schrempf's avatar Frieder Schrempf committed
      
      commit c898afdc upstream.
      
      Fix a use-after-free on dentry's d_fsdata fid list when a thread
      looks up a fid through dentry while another thread unlinks it:
      
      UAF thread:
      refcount_t: addition on 0; use-after-free.
       p9_fid_get linux/./include/net/9p/client.h:262
       v9fs_fid_find+0x236/0x280 linux/fs/9p/fid.c:129
       v9fs_fid_lookup_with_uid linux/fs/9p/fid.c:181
       v9fs_fid_lookup+0xbf/0xc20 linux/fs/9p/fid.c:314
       v9fs_vfs_getattr_dotl+0xf9/0x360 linux/fs/9p/vfs_inode_dotl.c:400
       vfs_statx+0xdd/0x4d0 linux/fs/stat.c:248
      
      Freed by:
       p9_fid_destroy (inlined)
       p9_client_clunk+0xb0/0xe0 linux/net/9p/client.c:1456
       p9_fid_put linux/./include/net/9p/client.h:278
       v9fs_dentry_release+0xb5/0x140 linux/fs/9p/vfs_dentry.c:55
       v9fs_remove+0x38f/0x620 linux/fs/9p/vfs_inode.c:518
       vfs_unlink+0x29a/0x810 linux/fs/namei.c:4335
      
      The problem is that d_fsdata was not accessed under d_lock, because
      d_release() normally is only called once the dentry is otherwise no
      longer accessible but since we also call it explicitly in v9fs_remove
      that lock is required:
      move the hlist out of the dentry under lock then unref its fids once
      they are no longer accessible.
      
      Fixes: 154372e6 ("fs/9p: fix create-unlink-getattr idiom")
      Cc: stable@vger.kernel.org
      Reported-by: Meysam Firouzi
      Reported-by: Amirmohammad Eftekhar
      Reviewed-by: default avatarChristian Schoenebeck <linux_oss@crudebyte.com>
      Message-ID: <20240521122947.1080227-1-asmadeus@codewreck.org>
      Signed-off-by: default avatarDominique Martinet <asmadeus@codewreck.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      d0b4e409
  4. Jun 12, 2024
  5. Feb 12, 2024
  6. Jan 23, 2024
    • David Howells's avatar
      mm, netfs, fscache: stop read optimisation when folio removed from pagecache · 8714d619
      David Howells authored and Frieder Schrempf's avatar Frieder Schrempf committed
      [ Upstream commit b4fa966f ]
      
      Fscache has an optimisation by which reads from the cache are skipped
      until we know that (a) there's data there to be read and (b) that data
      isn't entirely covered by pages resident in the netfs pagecache.  This is
      done with two flags manipulated by fscache_note_page_release():
      
      	if (...
      	    test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) &&
      	    test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags))
      		clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags);
      
      where the NO_DATA_TO_READ flag causes cachefiles_prepare_read() to
      indicate that netfslib should download from the server or clear the page
      instead.
      
      The fscache_note_page_release() function is intended to be called from
      ->releasepage() - but that only gets called if PG_private or PG_private_2
      is set - and currently the former is at the discretion of the network
      filesystem and the latter is only set whilst a page is being written to
      the cache, so sometimes we miss clearing the optimisation.
      
      Fix this by following Willy's suggestion[1] and adding an address_space
      flag, AS_RELEASE_ALWAYS, that causes filemap_release_folio() to always call
      ->release_folio() if it's set, even if PG_private or PG_private_2 aren't
      set.
      
      Note that this would require folio_test_private() and page_has_private() to
      become more complicated.  To avoid that, in the places[*] where these are
      used to conditionalise calls to filemap_release_folio() and
      try_to_release_page(), the tests are removed the those functions just
      jumped to unconditionally and the test is performed there.
      
      [*] There are some exceptions in vmscan.c where the check guards more than
      just a call to the releaser.  I've added a function, folio_needs_release()
      to wrap all the checks for that.
      
      AS_RELEASE_ALWAYS should be set if a non-NULL cookie is obtained from
      fscache and cleared in ->evict_inode() before truncate_inode_pages_final()
      is called.
      
      Additionally, the FSCACHE_COOKIE_NO_DATA_TO_READ flag needs to be cleared
      and the optimisation cancelled if a cachefiles object already contains data
      when we open it.
      
      [dwysocha@redhat.com: call folio_mapping() inside folio_needs_release()]
        Link: https://github.com/DaveWysochanskiRH/kernel/commit/902c990e311120179fa5de99d68364b2947b79ec
      Link: https://lkml.kernel.org/r/20230628104852.3391651-3-dhowells@redhat.com
      
      
      Fixes: 1f67e6d0 ("fscache: Provide a function to note the release of a page")
      Fixes: 047487c9 ("cachefiles: Implement the I/O routines")
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarDave Wysochanski <dwysocha@redhat.com>
      Reported-by: default avatarRohith Surabattula <rohiths.msft@gmail.com>
      Suggested-by: default avatarMatthew Wilcox <willy@infradead.org>
      Tested-by: default avatarSeongJae Park <sj@kernel.org>
      Cc: Daire Byrne <daire.byrne@gmail.com>
      Cc: Matthew Wilcox <willy@infradead.org>
      Cc: Linus Torvalds <torvalds@linux-foundation.org>
      Cc: Steve French <sfrench@samba.org>
      Cc: Shyam Prasad N <nspmangalore@gmail.com>
      Cc: Rohith Surabattula <rohiths.msft@gmail.com>
      Cc: Dave Wysochanski <dwysocha@redhat.com>
      Cc: Dominique Martinet <asmadeus@codewreck.org>
      Cc: Ilya Dryomov <idryomov@gmail.com>
      Cc: Andreas Dilger <adilger.kernel@dilger.ca>
      Cc: Jingbo Xu <jefflexu@linux.alibaba.com>
      Cc: "Theodore Ts'o" <tytso@mit.edu>
      Cc: Xiubo Li <xiubli@redhat.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Stable-dep-of: 1898efcd ("block: update the stable_writes flag in bdev_add")
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      8714d619
  7. Dec 12, 2023
  8. Feb 09, 2023
  9. Jul 02, 2022
  10. Jun 16, 2022
  11. Jun 15, 2022
  12. Jun 10, 2022
    • David Howells's avatar
      netfs: Rename the netfs_io_request cleanup op and give it an op pointer · 40a81101
      David Howells authored
      
      The netfs_io_request cleanup op is now always in a position to be given a
      pointer to a netfs_io_request struct, so this can be passed in instead of
      the mapping and private data arguments (both of which are included in the
      struct).
      
      So rename the ->cleanup op to ->free_request (to match ->init_request) and
      pass in the I/O pointer.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
      cc: linux-cachefs@redhat.com
      40a81101
    • Linus Torvalds's avatar
      netfs: Further cleanups after struct netfs_inode wrapper introduced · e81fb419
      Linus Torvalds authored
      
      Change the signature of netfs helper functions to take a struct netfs_inode
      pointer rather than a struct inode pointer where appropriate, thereby
      relieving the need for the network filesystem to convert its internal inode
      format down to the VFS inode only for netfslib to bounce it back up.  For
      type safety, it's better not to do that (and it's less typing too).
      
      Give netfs_write_begin() an extra argument to pass in a pointer to the
      netfs_inode struct rather than deriving it internally from the file
      pointer.  Note that the ->write_begin() and ->write_end() ops are intended
      to be replaced in the future by netfslib code that manages this without the
      need to call in twice for each page.
      
      netfs_readpage() and similar are intended to be pointed at directly by the
      address_space_operations table, so must stick to the signature dictated by
      the function pointers there.
      
      Changes
      =======
      - Updated the kerneldoc comments and documentation [DH].
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: linux-cachefs@redhat.com
      Link: https://lore.kernel.org/r/CAHk-=wgkwKyNmNdKpQkqZ6DnmUL-x9hp0YBnUGjaPFEAdxDTbw@mail.gmail.com/
      e81fb419
  13. Jun 09, 2022
    • David Howells's avatar
      netfs: Fix gcc-12 warning by embedding vfs inode in netfs_i_context · 874c8ca1
      David Howells authored
      
      While randstruct was satisfied with using an open-coded "void *" offset
      cast for the netfs_i_context <-> inode casting, __builtin_object_size() as
      used by FORTIFY_SOURCE was not as easily fooled.  This was causing the
      following complaint[1] from gcc v12:
      
        In file included from include/linux/string.h:253,
                         from include/linux/ceph/ceph_debug.h:7,
                         from fs/ceph/inode.c:2:
        In function 'fortify_memset_chk',
            inlined from 'netfs_i_context_init' at include/linux/netfs.h:326:2,
            inlined from 'ceph_alloc_inode' at fs/ceph/inode.c:463:2:
        include/linux/fortify-string.h:242:25: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
          242 |                         __write_overflow_field(p_size_field, size);
              |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      Fix this by embedding a struct inode into struct netfs_i_context (which
      should perhaps be renamed to struct netfs_inode).  The struct inode
      vfs_inode fields are then removed from the 9p, afs, ceph and cifs inode
      structs and vfs_inode is then simply changed to "netfs.inode" in those
      filesystems.
      
      Further, rename netfs_i_context to netfs_inode, get rid of the
      netfs_inode() function that converted a netfs_i_context pointer to an
      inode pointer (that can now be done with &ctx->inode) and rename the
      netfs_i_context() function to netfs_inode() (which is now a wrapper
      around container_of()).
      
      Most of the changes were done with:
      
        perl -p -i -e 's/vfs_inode/netfs.inode/'g \
              `git grep -l 'vfs_inode' -- fs/{9p,afs,ceph,cifs}/*.[ch]`
      
      Kees suggested doing it with a pair structure[2] and a special
      declarator to insert that into the network filesystem's inode
      wrapper[3], but I think it's cleaner to embed it - and then it doesn't
      matter if struct randomisation reorders things.
      
      Dave Chinner suggested using a filesystem-specific VFS_I() function in
      each filesystem to convert that filesystem's own inode wrapper struct
      into the VFS inode struct[4].
      
      Version #2:
       - Fix a couple of missed name changes due to a disabled cifs option.
       - Rename nfs_i_context to nfs_inode
       - Use "netfs" instead of "nic" as the member name in per-fs inode wrapper
         structs.
      
      [ This also undoes commit 507160f4 ("netfs: gcc-12: temporarily
        disable '-Wattribute-warning' for now") that is no longer needed ]
      
      Fixes: bc899ee1 ("netfs: Add a netfs inode context")
      Reported-by: default avatarJeff Layton <jlayton@kernel.org>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarXiubo Li <xiubli@redhat.com>
      cc: Jonathan Corbet <corbet@lwn.net>
      cc: Eric Van Hensbergen <ericvh@gmail.com>
      cc: Latchesar Ionkov <lucho@ionkov.net>
      cc: Dominique Martinet <asmadeus@codewreck.org>
      cc: Christian Schoenebeck <linux_oss@crudebyte.com>
      cc: Marc Dionne <marc.dionne@auristor.com>
      cc: Ilya Dryomov <idryomov@gmail.com>
      cc: Steve French <smfrench@gmail.com>
      cc: William Kucharski <william.kucharski@oracle.com>
      cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
      cc: Dave Chinner <david@fromorbit.com>
      cc: linux-doc@vger.kernel.org
      cc: v9fs-developer@lists.sourceforge.net
      cc: linux-afs@lists.infradead.org
      cc: ceph-devel@vger.kernel.org
      cc: linux-cifs@vger.kernel.org
      cc: samba-technical@lists.samba.org
      cc: linux-fsdevel@vger.kernel.org
      cc: linux-hardening@vger.kernel.org
      Link: https://lore.kernel.org/r/d2ad3a3d7bdd794c6efb562d2f2b655fb67756b9.camel@kernel.org/ [1]
      Link: https://lore.kernel.org/r/20220517210230.864239-1-keescook@chromium.org/ [2]
      Link: https://lore.kernel.org/r/20220518202212.2322058-1-keescook@chromium.org/ [3]
      Link: https://lore.kernel.org/r/20220524101205.GI2306852@dread.disaster.area/ [4]
      Link: https://lore.kernel.org/r/165296786831.3591209.12111293034669289733.stgit@warthog.procyon.org.uk/ # v1
      Link: https://lore.kernel.org/r/165305805651.4094995.7763502506786714216.stgit@warthog.procyon.org.uk
      
       # v2
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      874c8ca1
  14. May 10, 2022
  15. May 09, 2022
  16. May 08, 2022
  17. Mar 22, 2022
  18. Mar 18, 2022
  19. Mar 15, 2022
  20. Jan 30, 2022
    • Dominique Martinet's avatar
      Revert "fs/9p: search open fids first" · 22e424fe
      Dominique Martinet authored
      This reverts commit 478ba09e.
      
      That commit was meant as a fix for setattrs with by fd (e.g. ftruncate)
      to use an open fid instead of the first fid it found on lookup.
      The proper fix for that is to use the fid associated with the open file
      struct, available in iattr->ia_file for such operations, and was
      actually done just before in 66246641 ("9p: retrieve fid from file
      when file instance exist.")
      As such, this commit is no longer required.
      
      Furthermore, changing lookup to return open fids first had unwanted side
      effects, as it turns out the protocol forbids the use of open fids for
      further walks (e.g. clone_fid) and we broke mounts for some servers
      enforcing this rule.
      
      Note this only reverts to the old working behaviour, but it's still
      possible for lookup to return open fids if dentry->d_fsdata is not set,
      so more work is needed to make sure we respect this rule in the future,
      for example by adding a flag to the lookup functions to only match
      certain fid open modes depending on caller requirements.
      
      Link: https://lkml.kernel.org/r/20220130130651.712293-1-asmadeus@codewreck.org
      
      
      Fixes: 478ba09e ("fs/9p: search open fids first")
      Cc: stable@vger.kernel.org # v5.11+
      Reported-by: default avatarron minnich <rminnich@gmail.com>
      Reported-by: default avatar <ng@0x80.stream>
      Signed-off-by: default avatarDominique Martinet <asmadeus@codewreck.org>
      22e424fe
  21. Jan 11, 2022
Loading