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

Groups > linux.kernel > #1393415 > unrolled thread

Re: [PATCH] base: dd: don't remove driver_data in -EPROBE_DEFER case

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] base: dd: don't remove driver_data in -EPROBE_DEFER case Thierry Reding <thierry.reding@gmail.com> - 2016-05-03 15:20 +0200
    Re: [PATCH] base: dd: don't remove driver_data in -EPROBE_DEFER case Greg KH <gregkh@linuxfoundation.org> - 2016-05-03 17:10 +0200
      Re: [PATCH] base: dd: don't remove driver_data in -EPROBE_DEFER case Thierry Reding <thierry.reding@gmail.com> - 2016-05-03 17:20 +0200

#1393415 — Re: [PATCH] base: dd: don't remove driver_data in -EPROBE_DEFER case

[Multipart message — attachments visible in raw view] — view raw

On Tue, Mar 08, 2016 at 11:15:22PM +0800, Yi Zhang wrote:
> the driver_data may be used for sanity check, it fails the
> probe() if driver_data is NULL after it is re-triggered.
> for example, soc_probe() in sound/soc/soc-core.c
> 
> Signed-off-by: Yi Zhang <yizhang_hust@163.com>
> ---
>  drivers/base/dd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Hi Greg,

This causes a boot regression on at least one board, caused by one of
the drivers looking at driver data to check whether or not the driver
has properly loaded. If the code encounters a non-NULL pointer it
tries to dereference it, but because it's already been freed there is
no memory backing it and things crash.

I don't think keeping stale pointers around is a good idea. The whole
point of setting this to NULL in the core is so that probe failures
result in the same starting conditions no matter what.

Can we please get this reverted?

Cc'ing linux-tegra for visibility since that's where the boot regression
is observed.

Thierry

[toc] | [next] | [standalone]

#1393486

On Tue, May 03, 2016 at 03:11:26PM +0200, Thierry Reding wrote:
> On Tue, Mar 08, 2016 at 11:15:22PM +0800, Yi Zhang wrote:
> > the driver_data may be used for sanity check, it fails the
> > probe() if driver_data is NULL after it is re-triggered.
> > for example, soc_probe() in sound/soc/soc-core.c
> > 
> > Signed-off-by: Yi Zhang <yizhang_hust@163.com>
> > ---
> >  drivers/base/dd.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Hi Greg,
> 
> This causes a boot regression on at least one board, caused by one of
> the drivers looking at driver data to check whether or not the driver
> has properly loaded. If the code encounters a non-NULL pointer it
> tries to dereference it, but because it's already been freed there is
> no memory backing it and things crash.
> 
> I don't think keeping stale pointers around is a good idea. The whole
> point of setting this to NULL in the core is so that probe failures
> result in the same starting conditions no matter what.
> 
> Can we please get this reverted?
> 
> Cc'ing linux-tegra for visibility since that's where the boot regression
> is observed.

Now reverted, thanks for letting me know.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1393499

[Multipart message — attachments visible in raw view] — view raw

On Tue, May 03, 2016 at 08:07:39AM -0700, Greg KH wrote:
> On Tue, May 03, 2016 at 03:11:26PM +0200, Thierry Reding wrote:
> > On Tue, Mar 08, 2016 at 11:15:22PM +0800, Yi Zhang wrote:
> > > the driver_data may be used for sanity check, it fails the
> > > probe() if driver_data is NULL after it is re-triggered.
> > > for example, soc_probe() in sound/soc/soc-core.c
> > > 
> > > Signed-off-by: Yi Zhang <yizhang_hust@163.com>
> > > ---
> > >  drivers/base/dd.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Hi Greg,
> > 
> > This causes a boot regression on at least one board, caused by one of
> > the drivers looking at driver data to check whether or not the driver
> > has properly loaded. If the code encounters a non-NULL pointer it
> > tries to dereference it, but because it's already been freed there is
> > no memory backing it and things crash.
> > 
> > I don't think keeping stale pointers around is a good idea. The whole
> > point of setting this to NULL in the core is so that probe failures
> > result in the same starting conditions no matter what.
> > 
> > Can we please get this reverted?
> > 
> > Cc'ing linux-tegra for visibility since that's where the boot regression
> > is observed.
> 
> Now reverted, thanks for letting me know.

Thanks Greg.

Thierry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web