The patch titled knfsd: close a race-opportunity in d_splice_alias has been added to the -mm tree. Its filename is knfsd-close-a-race-opportunity-in-d_splice_alias.patch See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this ------------------------------------------------------ Subject: knfsd: close a race-opportunity in d_splice_alias From: NeilBrown <neilb@xxxxxxx> There is a possible race in d_splice_alias. Though __d_find_alias(inode, 1) will only return a dentry with DCACHE_DISCONNECTED set, it is possible for it to get cleared before the BUG_ON, and it is is not possible to lock against that. There are a couple of problems here. Firstly, the code doesn't match the comment. The comment describes a 'disconnected' dentry as being IS_ROOT as well as DCACHE_DISCONNECTED, however there is not testing of IS_ROOT anythere. A dentry is marked DCACHE_DISCONNECTED when allocated with d_alloc_anon, and remains DCACHE_DISCONNECTED while a path is built up towards the root. So a dentry can have a valid name and a valid parent and even grandparent, but will still be DCACHE_DISCONNECTED until a path to the root is created. Once the path to the root is complete, everything in the path gets DCACHE_DISCONNECTED cleared. So the fact that DCACHE_DISCONNECTED isn't enough to say that a dentry is free to be spliced in with a given name. This can only be allowed if the dentry does not yet have a name, so the IS_ROOT test is needed too. However even adding that test to __d_find_alias isn't enough. As d_splice_alias drops dcache_lock before calling d_move to perform the splice, it could race with another thread calling d_splice_alias to splice the inode in with a different name in a different part of the tree (in the case where a file has hard links). So that splicing code is only really safe for directories (as we know that directories only have one link). For directories, the caller of d_splice_alias will be holding i_mutex on the (unique) parent so there is no room for a race. A consequence of this is that a non-directory will never benefit from being spliced into a pre-exisiting dentry, but that isn't a problem. It is perfectly OK for a non-directory to have multiple dentries, some anonymous, some not. And the comment for d_splice_alias says that it only happens for directories anyway. Signed-off-by: Neil Brown <neilb@xxxxxxx> Cc: Christoph Hellwig <hch@xxxxxx> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Cc: Dipankar Sarma <dipankar@xxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxx> --- fs/dcache.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff -puN fs/dcache.c~knfsd-close-a-race-opportunity-in-d_splice_alias fs/dcache.c --- a/fs/dcache.c~knfsd-close-a-race-opportunity-in-d_splice_alias +++ a/fs/dcache.c @@ -291,9 +291,9 @@ struct dentry * dget_locked(struct dentr * it can be unhashed only if it has no children, or if it is the root * of a filesystem. * - * If the inode has a DCACHE_DISCONNECTED alias, then prefer + * If the inode has an IS_ROOT, DCACHE_DISCONNECTED alias, then prefer * any other hashed alias over that one unless @want_discon is set, - * in which case only return a DCACHE_DISCONNECTED alias. + * in which case only return an IS_ROOT, DCACHE_DISCONNECTED alias. */ static struct dentry * __d_find_alias(struct inode *inode, int want_discon) @@ -309,7 +309,8 @@ static struct dentry * __d_find_alias(st prefetch(next); alias = list_entry(tmp, struct dentry, d_alias); if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) { - if (alias->d_flags & DCACHE_DISCONNECTED) + if (IS_ROOT(alias) && + (alias->d_flags & DCACHE_DISCONNECTED)) discon_alias = alias; else if (!want_discon) { __dget_locked(alias); @@ -1134,7 +1135,7 @@ struct dentry *d_splice_alias(struct ino { struct dentry *new = NULL; - if (inode) { + if (inode && S_ISDIR(inode->i_mode)) { spin_lock(&dcache_lock); new = __d_find_alias(inode, 1); if (new) { _ Patches currently in -mm which might be from neilb@xxxxxxx are vfs-destroy-the-dentries-contributed-by-a-superblock-on-unmounting.patch knfsd-knfsd-add-some-missing-newlines-in-printks.patch knfsd-knfsd-remove-an-unused-variable-from-e_show.patch knfsd-knfsd-remove-an-unused-variable-from-auth_unix_lookup.patch knfsd-add-a-callback-for-when-last-rpc-thread-finishes.patch knfsd-add-a-callback-for-when-last-rpc-thread-finishes-tidy.patch knfsd-be-more-selective-in-which-sockets-lockd-listens-on.patch knfsd-remove-nfsd_versbits-as-intermediate-storage-for-desired-versions.patch knfsd-separate-out-some-parts-of-nfsd_svc-which-start-nfs-servers.patch knfsd-separate-out-some-parts-of-nfsd_svc-which-start-nfs-servers-tweaks.patch knfsd-define-new-nfsdfs-file-portlist-contains-list-of-ports.patch knfsd-define-new-nfsdfs-file-portlist-contains-list-of-ports-tidy.patch knfsd-define-new-nfsdfs-file-portlist-contains-list-of-ports-fix.patch knfsd-allow-sockets-to-be-passed-to-nfsd-via-portlist.patch knfsd-use-seq_start_token-instead-of-hardcoded-magic-void1.patch knfsd-drop-serv-option-to-svc_recv-and-svc_process.patch knfsd-drop-serv-option-to-svc_recv-and-svc_process-nfs-callback-fix-nfs-callback-fix.patch knfsd-check-return-value-of-lockd_up-in-write_ports.patch knfsd-move-makesock-failed-warning-into-make_socks.patch knfsd-correctly-handle-error-condition-from-lockd_up.patch knfsd-move-tempsock-aging-to-a-timer.patch knfsd-move-tempsock-aging-to-a-timer-tidy.patch knfsd-convert-sk_inuse-to-atomic_t.patch knfsd-use-new-lock-for-svc_sock-deferred-list.patch knfsd-convert-sk_reserved-to-atomic_t.patch knfsd-test-and-set-sk_busy-atomically.patch knfsd-split-svc_serv-into-pools.patch knfsd-split-svc_serv-into-pools-fix.patch knfsd-add-svc_get.patch knfsd-add-svc_set_num_threads.patch knfsd-use-svc_set_num_threads-to-manage-threads-in-knfsd.patch knfsd-make-rpc-threads-pools-numa-aware.patch knfsd-make-rpc-threads-pools-numa-aware-fix.patch knfsd-allow-admin-to-set-nthreads-per-node.patch nfsd-lockdep-annotation.patch knfsd-nfsd-lockdep-annotation-fix.patch knfsd-call-lockd_down-when-closing-a-socket-via-a-write-to-nfsd-portlist.patch knfsd-protect-update-to-sn_nrthreads-with-lock_kernel.patch knfsd-fixed-handling-of-lockd-fail-when-adding-nfsd-socket.patch knfsd-replace-two-page-lists-in-struct-svc_rqst-with-one.patch knfsd-replace-two-page-lists-in-struct-svc_rqst-with-one-fix.patch knfsd-avoid-excess-stack-usage-in-svc_tcp_recvfrom.patch knfsd-prepare-knfsd-for-support-of-rsize-wsize-of-up-to-1mb-over-tcp.patch knfsd-allow-max-size-of-nfsd-payload-to-be-configured.patch knfsd-make-nfsd-readahead-params-cache-smp-friendly.patch knfsd-knfsd-cache-ipmap-per-tcp-socket.patch knfsd-hide-use-of-lockds-h_monitored-flag.patch knfsd-consolidate-common-code-for-statd-lockd-notification.patch knfsd-when-looking-up-a-lockd-host-pass-hostname-length.patch knfsd-lockd-introduce-nsm_handle.patch knfsd-lockd-introduce-nsm_handle-fix.patch knfsd-misc-minor-fixes-indentation-changes.patch knfsd-lockd-make-nlm_host_rebooted-use-the-nsm_handle.patch knfsd-lockd-make-the-nsm-upcalls-use-the-nsm_handle.patch knfsd-lockd-make-the-hash-chains-use-a-hlist_node.patch knfsd-lockd-change-list-of-blocked-list-to-list_node.patch knfsd-change-nlm_file-to-use-a-hlist.patch knfsd-lockd-make-nlm_traverse_-more-flexible.patch knfsd-lockd-add-nlm_destroy_host.patch knfsd-simplify-nlmsvc_invalidate_all.patch knfsd-lockd-optionally-use-hostnames-for-identifying-peers.patch knfsd-make-nlmclnt_next_cookie-smp-safe.patch knfsd-match-granted_res-replies-using-cookies.patch knfsd-export-nsm_local_state-to-user-space-via-sysctl.patch knfsd-lockd-fix-use-of-h_nextrebind.patch knfsd-register-all-rpc-programs-with-portmapper-by-default.patch knfsd-lockd-introduce-nsm_handle-sem2mutex.patch knfsd-svcrpc-gss-factor-out-some-common-wrapping-code.patch knfsd-svcrpc-gss-fix-failure-on-svc_denied-in-integrity-case.patch knfsd-svcrpc-use-consistent-variable-name-for-the-reply-state.patch knfsd-nfsd4-refactor-exp_pseudoroot.patch knfsd-nfsd4-clean-up-exp_pseudoroot.patch knfsd-nfsd4-acls-relax-the-nfsv4-posix-mapping.patch knfsd-nfsd4-acls-fix-inheritance.patch knfsd-nfsd4-acls-simplify-nfs4_acl_nfsv4_to_posix-interface.patch knfsd-nfsd4-acls-fix-handling-of-zero-length-acls.patch knfsd-add-nfs-export-support-to-tmpfs.patch knfsd-lockd-fix-refount-on-nsm.patch knfsd-fix-auto-sizing-of-nfsd-request-reply-buffers.patch knfsd-close-a-race-opportunity-in-d_splice_alias.patch knfsd-nfsd-store-export-path-in-export.patch knfsd-nfsd4-fslocations-data-structures.patch knfsd-nfsd4-xdr-encoding-for-fs_locations.patch knfsd-nfsd4-actually-use-all-the-pieces-to-implement-referrals.patch md-the-scheduled-removal-of-the-start_array-ioctl-for-md.patch md-fix-a-comment-that-is-wrong-in-raid5h.patch md-factor-out-part-of-raid10d-into-a-separate-function.patch md-replace-magic-numbers-in-sb_dirty-with-well-defined-bit-flags.patch md-remove-the-working_disks-and-failed_disks-from-raid5-state-data.patch md-remove-working_disks-from-raid10-state.patch md-new-sysfs-interface-for-setting-bits-in-the-write-intent-bitmap.patch md-remove-unnecessary-variable-x-in-stripe_to_pdidx.patch md-factor-out-part-of-raid1d-into-a-separate-function.patch md-remove-working_disks-from-raid1-state-data.patch md-improve-locking-around-error-handling.patch md-define-backing_dev_infocongested_fn-for-raid0-and-linear.patch md-define-congested_fn-for-raid1-raid10-and-multipath.patch md-add-a-congested_fn-function-for-raid5-6.patch md-make-messages-about-resync-recovery-etc-more-specific.patch md-fix-duplicity-of-levels-in-mdtxt.patch md-remove-max_md_devs-which-is-an-arbitrary-limit.patch md-remove-experimental-classification-from-raid5-reshape.patch md-use-ffz-instead-of-find_first_set-to-convert-multiplier-to-shift.patch md-allow-set_bitmap_file-to-work-on-64bit-kernel-with-32bit-userspace.patch md-add-error-reporting-to-superblock-write-failure.patch md-dm-reduce-stack-usage-with-stacked-block-devices.patch - To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html