Re: [PATCH] clk: check ops pointer on clock register

[Michael Turquette]

Hi Jerome & Stephen,

On Mon, Dec 18, 2017 at 12:06 PM, Jerome Brunet <jbrunet@xxxxxxxxxxxx> 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