Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1585610

Re: [PATCH] switchtec: cleanup cdev init

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH] switchtec: cleanup cdev init Logan Gunthorpe <logang@deltatee.com> - 2017-02-18 21:50 +0100
  Re: [PATCH] switchtec: cleanup cdev init Dan Williams <dan.j.williams@intel.com> - 2017-02-19 22:50 +0100
    Re: [PATCH] switchtec: cleanup cdev init Logan Gunthorpe <logang@deltatee.com> - 2017-02-20 05:30 +0100
      Re: [PATCH] switchtec: cleanup cdev init Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-02-21 19:40 +0100

csiph-web