Re: [PATCH] switchtec: cleanup cdev init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 19, 2017 at 09:22:35PM -0700, Logan Gunthorpe wrote:

> Really, in any situation where there's a cdev and a device in the same
> structure, the life cycles of the two become linked but their reference
> counts are not and that is the problem here.

Yes, the cdev must hold a kref on the containing struct otherwise
userspace can trigger a use after free. This cannot be fixed with an
approach inside the open/release function either as the cdev core code
itself relies on the memory to exist.

I've suggested something like this before:

https://lkml.org/lkml/2015/7/8/1066

So I hope this will make it in, it is a step in the right direction.

If it does, would you make another patch to go further? I think
cdev_init should take enough arguments to hold the enclosing kref, API
wise there should be no API to init a cdev without the caller
specifying the enclosing struct's kref. That is the only way we will
stamp this bug-class out.

Eg look at kernel/time/posix-clock.c, it is wrong in the same way as
well - the kref_put in posix_clock_release is not enough to make it
work, clk->cdev is referenced after posix_clock_release returns by the
cdev core so this has a use-after-free.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux