Re: [PATCH 1/1] hwmon: (ina2xx) initialize mutex before locking
On Mon, Apr 16, 2018 at 10:53:23PM +0500, ahsan_huss...@mentor.com wrote:
> From: Ahsan Hussain
>
> Upstream commit 8d008c0c ("hwmon: (ina2xx) Make calibration register
> value fixed"), makes ina2xx_set_shunt() call mutex_lock on an
> un-initialized mutex. Initialize it prior so we don't get a NULL pointer
> dereference error.
>
> Fixes: 8d008c0c ("hwmon: (ina2xx) Make calibration register value fixed")
>
> Signed-off-by: Ahsan Hussain
Hmm ... the patch still does not apply ... having closer look
It appears that you don't use the mainline kernel. Commit 8d008c0c
is not upstream. It is in a stable release. Only which one ... ok,
I see it in linux-4.9.y. The offending commit is also in 4.4.y,
4.14.y, and 4.15.y. The fix needs to be pulled from upstream into
those stable releases. I'll send a request to Greg.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] hwmon: (ina2xx) initialize mutex before locking
On Mon, Apr 16, 2018 at 10:08:19PM +0500, Ahsan Hussain wrote:
>
> Upstream commit
>
> 8d008c0c ("hwmon: (ina2xx) Make calibration register value fixed")
>
This doesn't have to be on separate lines; as written, it just causes
confusion.
> makes ina2xx_set_shunt() call mutex_lock on an un-initialized mutex.
> Initialize it prior so we don't get a NULL pointer dereference error
>
> Signed-off-by: Ahsan Hussain
Good find, but your patch is corrupted to the point where any attenpt to
fix it up on my side failed. Please resend without corruption, and please
provide a Fixes: line.
Thanks,
Guenter
> ---
> drivers/hwmon/ina2xx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index a629f7c..1304f01 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -457,6 +457,8 @@ static int ina2xx_probe(struct i2c_client *client,
> val = INA2XX_RSHUNT_DEFAULT;
> }
> +mutex_init(&data->config_lock);
> +
> ina2xx_set_shunt(data, val);
> ina2xx_regmap_config.max_register = data->config->registers;
> @@ -473,8 +475,6 @@ static int ina2xx_probe(struct i2c_client *client,
> return -ENODEV;
> }
> -mutex_init(&data->config_lock);
> -
> data->groups[group++] = &ina2xx_group;
> if (id->driver_data == ina226)
> data->groups[group++] = &ina226_group;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] hwmon: (ina2xx) initialize mutex before locking
On Mon, Apr 16, 2018 at 10:48:03PM +0500, Ahsan Hussain wrote:
>
>
> On 04/16/2018 10:28 PM, Guenter Roeck wrote:
> > On Mon, Apr 16, 2018 at 10:08:19PM +0500, Ahsan Hussain wrote:
> >>
> >> Upstream commit
> >>
> >> 8d008c0c ("hwmon: (ina2xx) Make calibration register value fixed")
> >>
> > This doesn't have to be on separate lines; as written, it just causes
> > confusion.
> >
> Will do in my next email.
> >> makes ina2xx_set_shunt() call mutex_lock on an un-initialized mutex.
> >> Initialize it prior so we don't get a NULL pointer dereference error
> >>
> >> Signed-off-by: Ahsan Hussain
> >
> > Good find, but your patch is corrupted to the point where any attenpt to
> > fix it up on my side failed. Please resend without corruption, and please
> > provide a Fixes: line.
> Appologies. This must be Thunderbird, I've been trying to coax into
> doing otherwise. Will send the corrected patch shortly.
Best is to set up and use git send-email. Anyway, you can see the result
of the corruption at https://patchwork.kernel.org/patch/10343535/;
it appears that patchwork doesn't understand your patch either.
Guenter
> >
> > Thanks,
> > Guenter
> >
> >> ---
> >> drivers/hwmon/ina2xx.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> >> index a629f7c..1304f01 100644
> >> --- a/drivers/hwmon/ina2xx.c
> >> +++ b/drivers/hwmon/ina2xx.c
> >> @@ -457,6 +457,8 @@ static int ina2xx_probe(struct i2c_client *client,
> >>val = INA2XX_RSHUNT_DEFAULT;
> >>}
> >> + mutex_init(&data->config_lock);
> >> +
> >>ina2xx_set_shunt(data, val);
> >>ina2xx_regmap_config.max_register = data->config->registers;
> >> @@ -473,8 +475,6 @@ static int ina2xx_probe(struct i2c_client *client,
> >>return -ENODEV;
> >>}
> >> - mutex_init(&data->config_lock);
> >> -
> >>data->groups[group++] = &ina2xx_group;
> >>if (id->driver_data == ina226)
> >>data->groups[group++] = &ina226_group;
> >> --
> >> 2.7.4
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] hwmon: (ina2xx) initialize mutex before locking
On 04/16/2018 10:28 PM, Guenter Roeck wrote:
> On Mon, Apr 16, 2018 at 10:08:19PM +0500, Ahsan Hussain wrote:
>>
>> Upstream commit
>>
>> 8d008c0c ("hwmon: (ina2xx) Make calibration register value fixed")
>>
> This doesn't have to be on separate lines; as written, it just causes
> confusion.
>
Will do in my next email.
>> makes ina2xx_set_shunt() call mutex_lock on an un-initialized mutex.
>> Initialize it prior so we don't get a NULL pointer dereference error
>>
>> Signed-off-by: Ahsan Hussain
>
> Good find, but your patch is corrupted to the point where any attenpt to
> fix it up on my side failed. Please resend without corruption, and please
> provide a Fixes: line.
Appologies. This must be Thunderbird, I've been trying to coax into
doing otherwise. Will send the corrected patch shortly.
>
> Thanks,
> Guenter
>
>> ---
>> drivers/hwmon/ina2xx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
>> index a629f7c..1304f01 100644
>> --- a/drivers/hwmon/ina2xx.c
>> +++ b/drivers/hwmon/ina2xx.c
>> @@ -457,6 +457,8 @@ static int ina2xx_probe(struct i2c_client *client,
>> val = INA2XX_RSHUNT_DEFAULT;
>> }
>> + mutex_init(&data->config_lock);
>> +
>> ina2xx_set_shunt(data, val);
>> ina2xx_regmap_config.max_register = data->config->registers;
>> @@ -473,8 +475,6 @@ static int ina2xx_probe(struct i2c_client *client,
>> return -ENODEV;
>> }
>> - mutex_init(&data->config_lock);
>> -
>> data->groups[group++] = &ina2xx_group;
>> if (id->driver_data == ina226)
>> data->groups[group++] = &ina226_group;
>> --
>> 2.7.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
