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
