Re: [PATCH] RDMA/ocrdma: Suppress a compiler warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 25, 2018 at 3:16 AM, Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> On Tue, Jul 24, 2018 at 2:40 AM, Selvin Xavier
> <selvin.xavier@xxxxxxxxxxxx> wrote:
>> On Tue, Jul 24, 2018 at 3:03 AM, Jason Gunthorpe <jgg@xxxxxxxx> wrote:
>>> On Wed, Jul 18, 2018 at 09:01:35AM -0700, Bart Van Assche wrote:
>>>> This patch avoids that the following compiler warning is reported when
>>>> building with gcc 8 and W=1:
>>>>
>>>> In function 'ocrdma_mbx_get_ctrl_attribs',
>>>>     inlined from 'ocrdma_init_hw' at drivers/infiniband/hw/ocrdma/ocrdma_hw.c:3224:11:
>>>> drivers/infiniband/hw/ocrdma/ocrdma_hw.c:1368:3: warning: 'strncpy' output may be truncated copying 31 bytes from a string of length 31 [-Wstringop-truncation]
>>>>    strncpy(dev->model_number,
>>>>    ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>     hba_attribs->controller_model_number, 31);
>>>>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>
>>>> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxx>
>>>> Cc: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>
>>>> Cc: Devesh Sharma <devesh.sharma@xxxxxxxxxxxx>
>>>>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
>>>> index c6c87cba943b..e578281471af 100644
>>>> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
>>>> @@ -1365,8 +1365,9 @@ static int ocrdma_mbx_get_ctrl_attribs(struct ocrdma_dev *dev)
>>>>               dev->hba_port_num = (hba_attribs->ptpnum_maxdoms_hbast_cv &
>>>>                                       OCRDMA_HBA_ATTRB_PTNUM_MASK)
>>>>                                       >> OCRDMA_HBA_ATTRB_PTNUM_SHIFT;
>>>> -             strncpy(dev->model_number,
>>>> -                     hba_attribs->controller_model_number, 31);
>>>> +             strlcpy(dev->model_number,
>>>> +                     hba_attribs->controller_model_number,
>>>> +                     sizeof(dev->model_number));
>>>
>>> Yikes this code is weird. Why was it hard coded at 31 when both arrays
>>> are length 32? Selvin?
>>>
>> Seems like  a bug.  FW structure  defines model_number as 32 byte array.
>> But usually FW populates only 12 bytes out of 32 bytes for
>> model_number. So it was
>> never reported as an issue.
>
> So, the issue is the NULL termination, ideally we want to copy up to
> 32 bytes from the firmware into 33 bytes of destination array, so that
> there is no truncation and it is always null terminated.
>
> Unless you think we need to trust the firmware, then the strlcpy is fine.
>

We could trust FW. FW populates only 12 bytes and rest of the
bytes are all NULL. But i still  feel using strlcpy is better.
That would prevent any scenarios in which Firmware decides to use
all 32 bytes without a NULL termination. I dont think any ocrdma
adapters available today uses all 32 bytes. If it is there, I think
 It is okay to truncate model number by one character.

Thanks
Selvin

> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux