Re: [PATCH] Prevent crash on missing sysfs attribute group

[Eric W. Biederman]

Ingo Molnar <mingo@xxxxxxxxxx> writes:

> * Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>
>> Nacked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>> 
>> Bruno PrÃmont <bonbons@xxxxxxxxxxxxxxxxx> writes:
>> 
>> > Prevent kernel from crashing when a device is being registered with sysfs
>> > but has no (aka NULL) group attributes, but warn about it so calling path
>> > can get fixed.
>> 
>> The idea is reasonable but the implementation is horrible.
>> 
>> >> Will do - but the underlying generic bug should be fixed as 
>> >> well: we must not crash just because some attributes are missing 
>> >> in a rarely used sub-driver ...
>> >> 
>> >> We should WARN_ON(), etc. - but not crash.
>> 
>> FIX perf to include sanity checks.
>
> Huh, so put repeated, duplicated, inconsistently applied sanity 
> checks into dozens of sysfs attribute using kernel subsystems?
>
> Major FAIL, dude.


> Eric's rant about putting sanit checks at every usage site is 
> just crazy talk.

No.  I was not talking about every usage site.  I was talking about the
sites that are don't have a direct call chain to the sysfs methods and
instead do something clever that makes backtraces worthless.

In the normal case sysfs registration problems are simple to trace
back to their source because the backtrace points a finger at
the piece of code that when registering had a problem.

Unfortunately perf is built differently.  perf seems to be built to hide
who the idiot was who registered the wrong piece of code and despite
having a perfectly good backtrace and knowing it was perf the person
reporting the original bug still was going to need to do a bisect to
find the real culprit of the problem.

So I am asking that since perf is built in a way that actively makes
debugging these kinds of problems hard that you please add additional
debugging code to perf_pmu_register or some other better location so
that simply registering something buggy with perf will show the bug.

Either that or please fix perf events so that a backtrace is worth
something.

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