lkml.org 
[lkml]   [2017]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] clk: check ops pointer on clock register
Hi Jerome & Stephen,

On Mon, Dec 18, 2017 at 12:06 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Mon, 2017-12-18 at 11:03 -0800, Stephen Boyd wrote:
>> On 12/18, Jerome Brunet wrote:
>> > Nothing really prevents a provider from (trying to) register a clock
>> > without providing the clock ops structure.
>> >
>> > We do check the individual fields before using them, but not the
>> > structure pointer itself. This may have the usual nasty consequences when
>> > the pointer is dereferenced, mostly likely when checking one the field
>> > during the initialization.
>>
>> Yes, that nasty consequence should be a kernel oops,
>
> Precisely
>
>> and the
>> developer should notice that before submitting the driver for
>> inclusion.
>
> Agreed. But people may make mistakes, which is why (at least partly) we
> do checks, isn't it ?

Agreed the developers should test before submitting, but procedurally
generated clocks (e.g. registering clocks in a loop using a
predictable register map, etc) could lead to a situation where a
developer doesn't test every possible iteration.

Hypothetical, but easy easy easy to fix with Jerome's patch.

>
>> I don't think we really care to return an error here
>> if this happens.
>>
>
> I don't understand why we would let a oops happen when can catch the error
> properly ?
>

Agreed with Jerome on this one.

Let's flip it on its head: any downside to this patch? If not I can merge.

Regards,
Mike

\
 
 \ /
  Last update: 2017-12-18 21:13    [W:0.074 / U:1.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site