On 06/26/2018 01:35 PM, Breno Leitao wrote: The subject line should have been updated to [PATCH v2] to clue recipients to the fact that this is an updated version and not a resend or accidental duplicate send. > Currently an open firmware property is copied into partition_name variable > without keeping a room for \0. > > Later one, this variable (partition_name), which is 97 bytes long, is > strncpyed into ibmvcsci_host_data->madapter_info->partition_name, which > is 96 bytes long, possibly truncating it 'again' and removing the \0. > > This patch simply decreases the partition name to 96 and just copy using > strlcpy() which guarantees that the string is \0 terminated. I think there > is no issue if this there is a truncation in this very first copy, i.e, > when the open firmware property is read and copied into the driver for the > very first time; > > This issue also causes the following warning on GCC 8: > > drivers/scsi/ibmvscsi/ibmvscsi.c:281:2: warning: ‘strncpy’ output may be truncated copying 96 bytes from a string of length 96 [-Wstringop-truncation] > ... > inlined from ‘ibmvscsi_probe’ at drivers/scsi/ibmvscsi/ibmvscsi.c:2221:7: > drivers/scsi/ibmvscsi/ibmvscsi.c:265:3: warning: ‘strncpy’ specified bound 97 equals destination size [-Wstringop-truncation] > > CC: Bart Van Assche <bart.vanassche@xxxxxxx> > CC: Tyrel Datwyler <tyreld@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx> > --- Also, it is generally recommended that you record your revision history here for the readers/reviewers to quickly see what changed, and to make sure once the patch is pulled this info isn't included in the commit log. ie. Changes in v2: - Addressed Bart's comment by replacing strncpy() with strlcpy() Otherwise, Acked-by: Tyrel Datwyler <tyreld@xxxxxxxxxxxxxxxxxx> > drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 17df76f0be3c..67a2c844e30d 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -93,7 +93,7 @@ static int max_requests = IBMVSCSI_MAX_REQUESTS_DEFAULT; > static int max_events = IBMVSCSI_MAX_REQUESTS_DEFAULT + 2; > static int fast_fail = 1; > static int client_reserve = 1; > -static char partition_name[97] = "UNKNOWN"; > +static char partition_name[96] = "UNKNOWN"; > static unsigned int partition_number = -1; > static LIST_HEAD(ibmvscsi_head); > > @@ -262,7 +262,7 @@ static void gather_partition_info(void) > > ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL); > if (ppartition_name) > - strncpy(partition_name, ppartition_name, > + strlcpy(partition_name, ppartition_name, > sizeof(partition_name)); > p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL); > if (p_number_ptr) >
