Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks

Tejun Heo <[email protected]> writes:

> Eric W. Biederman wrote:
>>> 	/* Find the first parent which has valid dentry.
>>> 	 */
>>> 	dentry = NULL;
>>> 	cur = sd;
>>> 	while (!(dentry = __sysfs_get_dentry(sb, cur))) {
>>> 		if (cur->s_flags & SYSFS_FLAG_REMOVED) {
>>> 			dentry = ERR_PTR(-ENOENT);
>>> 			break;
>>> 		}
>>> 		cur = cur->s_parent;
>>> 	}
>> 
>> Here we depend on the fact that sysfs_root is pointed to
>> by sb->s_root so we know it will always have a dentry.
>
> Hmmm... dentry could be ERR_PTR(-ENOENT) here if the REMOVED flag test
> succeeded, right?

Ugh right.  Now that I don't have the locking it probably makes
sense to have that path just return or branch to the exit.

>
>>> 	/* from the found dentry, look up depth times */
>>> 	while (dentry->d_fsdata != sd) {
>
> And then dereferenced.  The REMOVED test should never succeed there, so
> we're probably in the clear but still the code looks a bit scary.

Agreed.  That is a bug.  Either the removed test needs to
be removed because it can't happen or that case needs to be handled.

Thanks for spotting this one.

Mental blind spots are the worst.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

---

• References:
  • [PATCH 11/25] sysfs: Remove s_dentry
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 12/25] sysfs: Introduce sysfs_rename_mutex
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 13/25] sysfs: Simply sysfs_get_dentry
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 14/25] sysfs: Don't use lookup_one_len_kern
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 15/25] vfs: Remove lookup_one_len_kern
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 16/25] sysfs: Support for preventing unmounts.
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 17/25] sysfs: Rewrite rename in terms of sysfs dirents
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 18/25] sysfs: Rewrite sysfs_move_dir in terms of sysfs dirents
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 19/25] sysfs: sysfs_get_dentry add a sb parameter
    • From: [email protected] (Eric W. Biederman)
  • [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: [email protected] (Eric W. Biederman)
  • Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: Tejun Heo <[email protected]>
  • Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: [email protected] (Eric W. Biederman)
  • Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: Tejun Heo <[email protected]>
  • Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: [email protected] (Eric W. Biederman)
  • Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
    • From: Tejun Heo <[email protected]>

• Prev by Date: Re: [patch 3/4] sdio: extend sdio_readsb() and friends to handle any length of buffer
• Next by Date: Re: [PATCH] Apply memory policies to top two highest zones when highest zone is ZONE_MOVABLE
• Previous by thread: Re: [PATCH 20/25] sysfs: Rename Support multiple superblocks
• Next by thread: Re: [PATCH 19/25] sysfs: sysfs_get_dentry add a sb parameter
• Index(es):
  • Date
  • Thread

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]