Re: [PATCH] scsi: mpt2sas: mpt2sas_base.c: Cleaning up missing null-terminate after strncpy call

[Rickard Strandqvist]

Hi

I did not know that Linux had strlcpy, much better choice, of course!

It was not clear that this was done in the code. And my thought was,
rather, just that you were not sure if there was a null character
because it used strlcpy. if this can not possibly happen would be
almost as well to switch to strcpy outright.

So what happens now?
Should I make a new patch, with strlcpy or just straight off with strcpy then?



Best regards
Rickard Strandqvist


2014-06-05 0:28 GMT+02:00 James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>:
> On Wed, 2014-06-04 at 23:36 +0200, Rickard Strandqvist wrote:
>> Added a guaranteed null-terminate after call to strncpy.
>>
>> This was partly found using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@xxxxxxxxxxxxxxxxxx>
>> ---
>>  drivers/scsi/mpt2sas/mpt2sas_base.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> index bde63f7..4c3eceb 100644
>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> @@ -2097,7 +2097,8 @@ _base_display_ioc_capabilities(struct MPT2SAS_ADAPTER *ioc)
>>       u32 bios_version;
>>
>>       bios_version = le32_to_cpu(ioc->bios_pg3.BiosVersion);
>> -     strncpy(desc, ioc->manu_pg0.ChipName, 16);
>> +     strncpy(desc, ioc->manu_pg0.ChipName, sizeof(desc));
>> +     desc[sizeof(desc) - 1] = '\0';
>
> There's no bug here because the specs define the ChipName field of the
> manufacturing page 0 to be 16 bytes and null terminated.  The nasty part
> is the way this driver is littered with magic numbers.
>
> James
>
>
--
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/