Bug 131356 - [tmpfs][patch] unlink(2) on tmpfs removs wrong files with hard-links
Summary: [tmpfs][patch] unlink(2) on tmpfs removs wrong files with hard-links
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.1-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-04 09:50 UTC by ota
Modified: 2009-02-22 10:40 UTC (History)
0 users

See Also:


Attachments
file.diff (4.05 KB, patch)
2009-02-04 09:50 UTC, ota
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ota 2009-02-04 09:50:02 UTC
rm/unlink removes the first i-node entry specified but not by the same.  In the case below, "rm b" deletes file "a" where "a" and "b" are hard-linked and "a" is created first.

Fix: Patch is attached.  It replaces tmpfs_dir_search with a new version of tmpfs_dir_lookup.

Patch attached with submission follows:
How-To-Repeat: % cat unlink.sh          
#!/bin/sh

echo @ cleaning
rm -rf a b
echo @ file is created and linked
echo "123" > a
ln a b
echo @ a and b should exist
ls -lsa
head a b
echo @ b is removed
#unlink b
rm b
echo @ only a should exist
ls -lsa
head a b


On TMPFS:
% cd /mnt/tmpfs
% sh unlink.sh
@ cleaning
@ file is created and linked
@ a and b should exist
total 18
4 drwxrwxrwx   3 root  wheel   80  1 31 04:26 .
2 drwxr-xr-x  19 root  wheel  512 12 29 10:07 ..
4 -rw-r--r--   2 uyota wheel    4  1 31 04:26 a
4 -rw-r--r--   2 uyota wheel    4  1 31 04:26 b
==> a <==
123

==> b <==
123
@ b is removed
@ only a should exist
total 14
4 drwxrwxrwx   3 root  wheel   60  1 31 04:26 .
2 drwxr-xr-x  19 root  wheel  512 12 29 10:07 ..
4 -rw-r--r--   1 uyota wheel    4  1 31 04:26 b
==> a <==
123

==> b <==
123

On UFS:
% cd /mnt/ufs
% sh unlink.sh
@ cleaning
@ file is created and linked
@ a and b should exist
total 8
2 drwxr-xr-x   2 uyota wheel  512  1 31 04:27 .
2 drwxrwxrwt  15 root  wheel  512  1 31 04:27 ..
2 -rw-r--r--   2 uyota wheel    4  1 31 04:27 a
2 -rw-r--r--   2 uyota wheel    4  1 31 04:27 b
==> a <==
123

==> b <==
123
@ b is removed
@ only a should exist
total 6
2 drwxr-xr-x   2 uyota wheel  512  1 31 04:27 .
2 drwxrwxrwt  15 root  wheel  512  1 31 04:27 ..
2 -rw-r--r--   1 uyota wheel    4  1 31 04:27 a
==> a <==
123
Comment 1 Gavin Atkinson freebsd_committer freebsd_triage 2009-02-04 13:13:39 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s)
Comment 2 Konstantin Belousov freebsd_committer freebsd_triage 2009-02-08 13:27:57 UTC
Responsible Changed
From-To: freebsd-fs->kib

Take.
Comment 3 dfilter service freebsd_committer freebsd_triage 2009-02-08 19:18:47 UTC
Author: kib
Date: Sun Feb  8 19:18:33 2009
New Revision: 188318
URL: http://svn.freebsd.org/changeset/base/188318

Log:
  Lookup up the directory entry for the tmpfs node that are deleted by
  both node pointer and name component. This does the right thing for
  hardlinks to the same node in the same directory.
  
  Submitted by:	Yoshihiro Ota <ota j email ne jp>
  PR:	kern/131356
  MFC after:	2 weeks

Modified:
  head/sys/fs/tmpfs/tmpfs.h
  head/sys/fs/tmpfs/tmpfs_subr.c
  head/sys/fs/tmpfs/tmpfs_vnops.c

Modified: head/sys/fs/tmpfs/tmpfs.h
==============================================================================
--- head/sys/fs/tmpfs/tmpfs.h	Sun Feb  8 17:49:32 2009	(r188317)
+++ head/sys/fs/tmpfs/tmpfs.h	Sun Feb  8 19:18:33 2009	(r188318)
@@ -408,9 +408,8 @@ int	tmpfs_alloc_file(struct vnode *, str
 void	tmpfs_dir_attach(struct vnode *, struct tmpfs_dirent *);
 void	tmpfs_dir_detach(struct vnode *, struct tmpfs_dirent *);
 struct tmpfs_dirent *	tmpfs_dir_lookup(struct tmpfs_node *node,
+			    struct tmpfs_node *f,
 			    struct componentname *cnp);
-struct tmpfs_dirent *tmpfs_dir_search(struct tmpfs_node *node,
-    struct tmpfs_node *f);
 int	tmpfs_dir_getdotdent(struct tmpfs_node *, struct uio *);
 int	tmpfs_dir_getdotdotdent(struct tmpfs_node *, struct uio *);
 struct tmpfs_dirent *	tmpfs_dir_lookupbycookie(struct tmpfs_node *, off_t);

Modified: head/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_subr.c	Sun Feb  8 17:49:32 2009	(r188317)
+++ head/sys/fs/tmpfs/tmpfs_subr.c	Sun Feb  8 19:18:33 2009	(r188318)
@@ -571,7 +571,8 @@ tmpfs_dir_detach(struct vnode *vp, struc
  * Returns a pointer to the entry when found, otherwise NULL.
  */
 struct tmpfs_dirent *
-tmpfs_dir_lookup(struct tmpfs_node *node, struct componentname *cnp)
+tmpfs_dir_lookup(struct tmpfs_node *node, struct tmpfs_node *f,
+    struct componentname *cnp)
 {
 	boolean_t found;
 	struct tmpfs_dirent *de;
@@ -583,6 +584,8 @@ tmpfs_dir_lookup(struct tmpfs_node *node
 
 	found = 0;
 	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
+		if (f != NULL && de->td_node != f)
+		    continue;
 		MPASS(cnp->cn_namelen < 0xffff);
 		if (de->td_namelen == (uint16_t)cnp->cn_namelen &&
 		    bcmp(de->td_name, cnp->cn_nameptr, de->td_namelen) == 0) {
@@ -595,20 +598,6 @@ tmpfs_dir_lookup(struct tmpfs_node *node
 	return found ? de : NULL;
 }
 
-struct tmpfs_dirent *
-tmpfs_dir_search(struct tmpfs_node *node, struct tmpfs_node *f)
-{
-	struct tmpfs_dirent *de;
-
-	TMPFS_VALIDATE_DIR(node);
-	node->tn_status |= TMPFS_NODE_ACCESSED;
-	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
-		if (de->td_node == f)
-			return (de);
-	}
-	return (NULL);
-}
-
 /* --------------------------------------------------------------------- */
 
 /*

Modified: head/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_vnops.c	Sun Feb  8 17:49:32 2009	(r188317)
+++ head/sys/fs/tmpfs/tmpfs_vnops.c	Sun Feb  8 19:18:33 2009	(r188318)
@@ -104,7 +104,7 @@ tmpfs_lookup(struct vop_cachedlookup_arg
 		*vpp = dvp;
 		error = 0;
 	} else {
-		de = tmpfs_dir_lookup(dnode, cnp);
+		de = tmpfs_dir_lookup(dnode, NULL, cnp);
 		if (de == NULL) {
 			/* The entry was not found in the directory.
 			 * This is OK if we are creating or renaming an
@@ -772,7 +772,7 @@ tmpfs_remove(struct vop_remove_args *v)
 	dnode = VP_TO_TMPFS_DIR(dvp);
 	node = VP_TO_TMPFS_NODE(vp);
 	tmp = VFS_TO_TMPFS(vp->v_mount);
-	de = tmpfs_dir_search(dnode, node);
+	de = tmpfs_dir_lookup(dnode, node, v->a_cnp);
 	MPASS(de != NULL);
 
 	/* Files marked as immutable or append-only cannot be deleted. */
@@ -919,7 +919,7 @@ tmpfs_rename(struct vop_rename_args *v)
 	}
 	fdnode = VP_TO_TMPFS_DIR(fdvp);
 	fnode = VP_TO_TMPFS_NODE(fvp);
-	de = tmpfs_dir_search(fdnode, fnode);
+	de = tmpfs_dir_lookup(fdnode, fnode, fcnp);
 
 	/* Avoid manipulating '.' and '..' entries. */
 	if (de == NULL) {
@@ -1031,7 +1031,7 @@ tmpfs_rename(struct vop_rename_args *v)
 	 * from the target directory. */
 	if (tvp != NULL) {
 		/* Remove the old entry from the target directory. */
-		de = tmpfs_dir_search(tdnode, tnode);
+		de = tmpfs_dir_lookup(tdnode, tnode, tcnp);
 		tmpfs_dir_detach(tdvp, de);
 
 		/* Free the directory entry we just deleted.  Note that the
@@ -1119,7 +1119,7 @@ tmpfs_rmdir(struct vop_rmdir_args *v)
 
 	/* Get the directory entry associated with node (vp).  This was
 	 * filled by tmpfs_lookup while looking up the entry. */
-	de = tmpfs_dir_search(dnode, node);
+	de = tmpfs_dir_lookup(dnode, node, v->a_cnp);
 	MPASS(TMPFS_DIRENT_MATCHES(de,
 	    v->a_cnp->cn_nameptr,
 	    v->a_cnp->cn_namelen));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 4 dfilter service freebsd_committer freebsd_triage 2009-02-22 10:25:31 UTC
Author: kib
Date: Sun Feb 22 10:25:07 2009
New Revision: 188911
URL: http://svn.freebsd.org/changeset/base/188911

Log:
  MFC r188318:
  Lookup up the directory entry for the tmpfs node that are deleted by
  both node pointer and name component. This does the right thing for
  hardlinks to the same node in the same directory.
  
  PR:   kern/131356

Modified:
  stable/7/sys/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)
  stable/7/sys/dev/ath/ath_hal/   (props changed)
  stable/7/sys/dev/cxgb/   (props changed)
  stable/7/sys/fs/tmpfs/tmpfs.h
  stable/7/sys/fs/tmpfs/tmpfs_subr.c
  stable/7/sys/fs/tmpfs/tmpfs_vnops.c

Modified: stable/7/sys/fs/tmpfs/tmpfs.h
==============================================================================
--- stable/7/sys/fs/tmpfs/tmpfs.h	Sun Feb 22 10:17:10 2009	(r188910)
+++ stable/7/sys/fs/tmpfs/tmpfs.h	Sun Feb 22 10:25:07 2009	(r188911)
@@ -408,9 +408,8 @@ int	tmpfs_alloc_file(struct vnode *, str
 void	tmpfs_dir_attach(struct vnode *, struct tmpfs_dirent *);
 void	tmpfs_dir_detach(struct vnode *, struct tmpfs_dirent *);
 struct tmpfs_dirent *	tmpfs_dir_lookup(struct tmpfs_node *node,
+			    struct tmpfs_node *f,
 			    struct componentname *cnp);
-struct tmpfs_dirent *tmpfs_dir_search(struct tmpfs_node *node,
-    struct tmpfs_node *f);
 int	tmpfs_dir_getdotdent(struct tmpfs_node *, struct uio *);
 int	tmpfs_dir_getdotdotdent(struct tmpfs_node *, struct uio *);
 struct tmpfs_dirent *	tmpfs_dir_lookupbycookie(struct tmpfs_node *, off_t);

Modified: stable/7/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- stable/7/sys/fs/tmpfs/tmpfs_subr.c	Sun Feb 22 10:17:10 2009	(r188910)
+++ stable/7/sys/fs/tmpfs/tmpfs_subr.c	Sun Feb 22 10:25:07 2009	(r188911)
@@ -572,7 +572,8 @@ tmpfs_dir_detach(struct vnode *vp, struc
  * Returns a pointer to the entry when found, otherwise NULL.
  */
 struct tmpfs_dirent *
-tmpfs_dir_lookup(struct tmpfs_node *node, struct componentname *cnp)
+tmpfs_dir_lookup(struct tmpfs_node *node, struct tmpfs_node *f,
+    struct componentname *cnp)
 {
 	boolean_t found;
 	struct tmpfs_dirent *de;
@@ -584,6 +585,8 @@ tmpfs_dir_lookup(struct tmpfs_node *node
 
 	found = 0;
 	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
+		if (f != NULL && de->td_node != f)
+		    continue;
 		MPASS(cnp->cn_namelen < 0xffff);
 		if (de->td_namelen == (uint16_t)cnp->cn_namelen &&
 		    memcmp(de->td_name, cnp->cn_nameptr, de->td_namelen) == 0) {
@@ -596,20 +599,6 @@ tmpfs_dir_lookup(struct tmpfs_node *node
 	return found ? de : NULL;
 }
 
-struct tmpfs_dirent *
-tmpfs_dir_search(struct tmpfs_node *node, struct tmpfs_node *f)
-{
-	struct tmpfs_dirent *de;
-
-	TMPFS_VALIDATE_DIR(node);
-	node->tn_status |= TMPFS_NODE_ACCESSED;
-	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
-		if (de->td_node == f)
-			return (de);
-	}
-	return (NULL);
-}
-
 /* --------------------------------------------------------------------- */
 
 /*

Modified: stable/7/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- stable/7/sys/fs/tmpfs/tmpfs_vnops.c	Sun Feb 22 10:17:10 2009	(r188910)
+++ stable/7/sys/fs/tmpfs/tmpfs_vnops.c	Sun Feb 22 10:25:07 2009	(r188911)
@@ -104,7 +104,7 @@ tmpfs_lookup(struct vop_cachedlookup_arg
 		*vpp = dvp;
 		error = 0;
 	} else {
-		de = tmpfs_dir_lookup(dnode, cnp);
+		de = tmpfs_dir_lookup(dnode, NULL, cnp);
 		if (de == NULL) {
 			/* The entry was not found in the directory.
 			 * This is OK if we are creating or renaming an
@@ -771,7 +771,7 @@ tmpfs_remove(struct vop_remove_args *v)
 	dnode = VP_TO_TMPFS_DIR(dvp);
 	node = VP_TO_TMPFS_NODE(vp);
 	tmp = VFS_TO_TMPFS(vp->v_mount);
-	de = tmpfs_dir_search(dnode, node);
+	de = tmpfs_dir_lookup(dnode, node, v->a_cnp);
 	MPASS(de != NULL);
 
 	/* Files marked as immutable or append-only cannot be deleted. */
@@ -918,7 +918,7 @@ tmpfs_rename(struct vop_rename_args *v)
 	}
 	fdnode = VP_TO_TMPFS_DIR(fdvp);
 	fnode = VP_TO_TMPFS_NODE(fvp);
-	de = tmpfs_dir_search(fdnode, fnode);
+	de = tmpfs_dir_lookup(fdnode, fnode, fcnp);
 
 	/* Avoid manipulating '.' and '..' entries. */
 	if (de == NULL) {
@@ -1030,7 +1030,7 @@ tmpfs_rename(struct vop_rename_args *v)
 	 * from the target directory. */
 	if (tvp != NULL) {
 		/* Remove the old entry from the target directory. */
-		de = tmpfs_dir_search(tdnode, tnode);
+		de = tmpfs_dir_lookup(tdnode, tnode, tcnp);
 		tmpfs_dir_detach(tdvp, de);
 
 		/* Free the directory entry we just deleted.  Note that the
@@ -1118,7 +1118,7 @@ tmpfs_rmdir(struct vop_rmdir_args *v)
 
 	/* Get the directory entry associated with node (vp).  This was
 	 * filled by tmpfs_lookup while looking up the entry. */
-	de = tmpfs_dir_search(dnode, node);
+	de = tmpfs_dir_lookup(dnode, node, v->a_cnp);
 	MPASS(TMPFS_DIRENT_MATCHES(de,
 	    v->a_cnp->cn_nameptr,
 	    v->a_cnp->cn_namelen));
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 Konstantin Belousov freebsd_committer freebsd_triage 2009-02-22 10:30:00 UTC
State Changed
From-To: open->closed

Committed to HEAD and RELENG_7, thanks.