vfs: sanitize __d_path()
in [Kernel]
|
Prev: [PATCH 4/11] drivers/net/hamradio: Eliminate a NULL pointer dereference
Next: [PATCH 11/11] drivers/usb/serial: Eliminate a NULL pointer dereference
From: Alexey Dobriyan on 27 May 2010 08:40 On Thu, May 27, 2010 at 1:29 PM, Miklos Szeredi <miklos(a)szeredi.hu> wrote: > From: Miklos Szeredi <mszeredi(a)suse.cz> > > __d_path() no longer appends " (deleted)" to unlinked paths. �This is > moved into d_path() which is the only caller that cares. d_path() or equivalent should get "int *deleted" argument to distinguish really deleted files. Then users can decide if they care or not. > If a global root is reached then __d_path() no longer prepends the > name of the root dentry. �This was needed by pseudo filesystems, but > the d_dname() method has superseded it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Miklos Szeredi on 27 May 2010 08:50 On Thu, 27 May 2010, Alexey Dobriyan wrote: > On Thu, May 27, 2010 at 1:29 PM, Miklos Szeredi <miklos(a)szeredi.hu> wrote: > > From: Miklos Szeredi <mszeredi(a)suse.cz> > > > > __d_path() no longer appends " (deleted)" to unlinked paths. This is > > moved into d_path() which is the only caller that cares. > > d_path() or equivalent should get "int *deleted" argument to distinguish > really deleted files. Then users can decide if they care or not. Why can't they distinguish deleted files by just calling d_unlinked()? Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alexey Dobriyan on 27 May 2010 09:30 On Thu, May 27, 2010 at 3:47 PM, Miklos Szeredi <miklos(a)szeredi.hu> wrote: > On Thu, 27 May 2010, Alexey Dobriyan wrote: >> On Thu, May 27, 2010 at 1:29 PM, Miklos Szeredi <miklos(a)szeredi.hu> wrote: >> > From: Miklos Szeredi <mszeredi(a)suse.cz> >> > >> > __d_path() no longer appends " (deleted)" to unlinked paths. �This is >> > moved into d_path() which is the only caller that cares. >> >> d_path() or equivalent should get "int *deleted" argument to distinguish >> really deleted files. Then users can decide if they care or not. > > Why can't they distinguish deleted files by just calling d_unlinked()? Why would they want to do it (which means taking locks again and potential incoherence)? The information is right there, ship it upwards: + if (deleted) + *deleted = 0; spin_lock(&vfsmount_lock); prepend(&end, &buflen, "\0", 1); - if (d_unlinked(dentry) && - (prepend(&end, &buflen, " (deleted)", 10) != 0)) - goto Elong; + if (d_unlinked(dentry) && deleted) + *deleted = 1; "(deleted)" as interface sucks, we can't change it, at least, let's make in-kernel interface correct. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tetsuo Handa on 27 May 2010 09:50 Alexey Dobriyan wrote: > Why would they want to do it (which means taking locks again and > potential incoherence)? > The information is right there, ship it upwards: > > + if (deleted) > + *deleted = 0; > spin_lock(&vfsmount_lock); > prepend(&end, &buflen, "\0", 1); > - if (d_unlinked(dentry) && > - (prepend(&end, &buflen, " (deleted)", 10) != 0)) > - goto Elong; > + if (d_unlinked(dentry) && deleted) > + *deleted = 1; > > "(deleted)" as interface sucks, we can't change it, > at least, let's make in-kernel interface correct. We don't need vfsmount_lock for d_unlinked(), do we? Then, I think we can do + if (deleted) + *deleted = d_unlinked(dentry); spin_lock(&vfsmount_lock); - prepend(&end, &buflen, "\0", 1); + prepend(&end, &buflen, "", 1); - if (d_unlinked(dentry) && - (prepend(&end, &buflen, " (deleted)", 10) != 0)) - goto Elong; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Miklos Szeredi on 27 May 2010 09:50
On Thu, 27 May 2010, Alexey Dobriyan wrote: > Why would they want to do it (which means taking locks again and > potential incoherence)? > The information is right there, ship it upwards: > > + if (deleted) > + *deleted = 0; > spin_lock(&vfsmount_lock); > prepend(&end, &buflen, "\0", 1); > - if (d_unlinked(dentry) && > - (prepend(&end, &buflen, " (deleted)", 10) != 0)) > - goto Elong; > + if (d_unlinked(dentry) && deleted) > + *deleted = 1; > > "(deleted)" as interface sucks, we can't change it, > at least, let's make in-kernel interface correct. > Do you have any specific caller in mind? The above will just result in extra code in most callers. Better have one function with does the old "(deleted)" thing, and one which doesn't. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |