* [PATCH 0/3] mdadm: Fix some building errors @ 2017-03-17 11:55 Xiao Ni 2017-03-17 11:55 ` [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation Xiao Ni ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw) To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz There are some error buildings in Fedora release 26 (Rawhide) The gcc version is gcc (GCC) 7.0.1 20170211 (Red Hat 7.0.1-0.8) There are three types of errors: 1. Fall through mdadm.c:149:28: error: this statement may fall through [-Werror=implicit-fallthrough=] if (mode == ASSEMBLE || mode == BUILD || ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ mode == CREATE || mode == GROW || ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ mode == INCREMENTAL || mode == MANAGE) ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~ mdadm.c:151:3: note: here case Brief: ^~~~ 2. format-overflow Detail.c: In function ‘Detail’: Detail.c:584:31: error: ‘%s’ directive writing up to 255 bytes into a region of size 189 [-Werror=format-overflow=] sprintf(path, "/sys/block/%s/md/metadata_version", ^~ Detail.c:584:5: note: ‘sprintf’ output between 32 and 287 bytes into a destination of size 200 sprintf(path, "/sys/block/%s/md/metadata_version", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ de->d_name); ~~~~~~~~~~~ 3. format-truncation super-intel.c: In function ‘examine_super_imsm’: super-intel.c:1815:30: error: ‘%s’ directive output may be truncated writing up to 31 bytes into a region of size 24 [-Werror=format-truncation=] snprintf(str, MPB_SIG_LEN, "%s", mpb->sig); ^~ super-intel.c:1815:2: note: ‘snprintf’ output between 1 and 32 bytes into a destination of size 24 snprintf(str, MPB_SIG_LEN, "%s", mpb->sig); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Xiao Ni (3): Replace snprintf with strncpy at some places to avoid truncation Add Wimplicit-fallthrough=0 in Makefile Specify enough length when write to buffer Detail.c | 2 +- Makefile | 5 +++++ super-intel.c | 11 +++++++---- super0.c | 2 +- util.c | 2 +- 5 files changed, 15 insertions(+), 7 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation 2017-03-17 11:55 [PATCH 0/3] mdadm: Fix some building errors Xiao Ni @ 2017-03-17 11:55 ` Xiao Ni 2017-03-17 19:55 ` jes.sorensen 2017-03-17 11:55 ` [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile Xiao Ni 2017-03-17 11:55 ` [PATCH 3/3] mdadm: Specify enough length when write to buffer Xiao Ni 2 siblings, 1 reply; 8+ messages in thread From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw) To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz In gcc7 there are some building errors like: directive output may be truncated writing up to 31 bytes into a region of size 24 snprintf(str, MPB_SIG_LEN, %s, mpb->sig); It just need to copy one string to target. So use strncpy to replace it. Signed-off-by: Xiao Ni <xni@redhat.com> --- super-intel.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/super-intel.c b/super-intel.c index d5e9517..e1618f1 100644 --- a/super-intel.c +++ b/super-intel.c @@ -1811,7 +1811,8 @@ static void examine_super_imsm(struct supertype *st, char *homehost) __u32 reserved = imsm_reserved_sectors(super, super->disks); struct dl *dl; - snprintf(str, MPB_SIG_LEN, "%s", mpb->sig); + strncpy(str, (char *)mpb->sig, MPB_SIG_LEN); + str[MPB_SIG_LEN-1] = '\0'; printf(" Magic : %s\n", str); snprintf(str, strlen(MPB_VERSION_RAID0), "%s", get_imsm_version(mpb)); printf(" Version : %s\n", get_imsm_version(mpb)); @@ -5227,7 +5228,7 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info, disk->status = CONFIGURED_DISK | FAILED_DISK; disk->scsi_id = __cpu_to_le32(~(__u32)0); snprintf((char *) disk->serial, MAX_RAID_SERIAL_LEN, - "missing:%d", i); + "missing:%d", (__u8)i); } find_missing(super); } else { @@ -7142,14 +7143,16 @@ static int update_subarray_imsm(struct supertype *st, char *subarray, u->type = update_rename_array; u->dev_idx = vol; - snprintf((char *) u->name, MAX_RAID_SERIAL_LEN, "%s", name); + strncpy((char *) u->name, name, MAX_RAID_SERIAL_LEN); + u->name[MAX_RAID_SERIAL_LEN-1] = '\0'; append_metadata_update(st, u, sizeof(*u)); } else { struct imsm_dev *dev; int i; dev = get_imsm_dev(super, vol); - snprintf((char *) dev->volume, MAX_RAID_SERIAL_LEN, "%s", name); + strncpy((char *) dev->volume, name, MAX_RAID_SERIAL_LEN); + dev->volume[MAX_RAID_SERIAL_LEN-1] = '\0'; for (i = 0; i < mpb->num_raid_devs; i++) { dev = get_imsm_dev(super, i); handle_missing(super, dev); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation 2017-03-17 11:55 ` [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation Xiao Ni @ 2017-03-17 19:55 ` jes.sorensen 2017-03-18 1:02 ` Xiao Ni 0 siblings, 1 reply; 8+ messages in thread From: jes.sorensen @ 2017-03-17 19:55 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz Xiao Ni <xni@redhat.com> writes: > In gcc7 there are some building errors like: > directive output may be truncated writing up to 31 bytes into a region of size 24 > snprintf(str, MPB_SIG_LEN, %s, mpb->sig); > > It just need to copy one string to target. So use strncpy to replace it. > > Signed-off-by: Xiao Ni <xni@redhat.com> > --- > super-intel.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/super-intel.c b/super-intel.c > index d5e9517..e1618f1 100644 > --- a/super-intel.c > +++ b/super-intel.c > @@ -1811,7 +1811,8 @@ static void examine_super_imsm(struct supertype *st, char *homehost) > __u32 reserved = imsm_reserved_sectors(super, super->disks); > struct dl *dl; > > - snprintf(str, MPB_SIG_LEN, "%s", mpb->sig); > + strncpy(str, (char *)mpb->sig, MPB_SIG_LEN); > + str[MPB_SIG_LEN-1] = '\0'; > printf(" Magic : %s\n", str); > snprintf(str, strlen(MPB_VERSION_RAID0), "%s", get_imsm_version(mpb)); > printf(" Version : %s\n", get_imsm_version(mpb)); Given str is defined as 'char str[MAX_SIGNATURE_LENGTH]', it would be more correct to use MAX_SIGNATURE_LENGTH as the size limit here. > @@ -5227,7 +5228,7 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info, > disk->status = CONFIGURED_DISK | FAILED_DISK; > disk->scsi_id = __cpu_to_le32(~(__u32)0); > snprintf((char *) disk->serial, MAX_RAID_SERIAL_LEN, > - "missing:%d", i); > + "missing:%d", (__u8)i); > } > find_missing(super); > } else { This is unrelated to the commit message. > @@ -7142,14 +7143,16 @@ static int update_subarray_imsm(struct supertype *st, char *subarray, > > u->type = update_rename_array; > u->dev_idx = vol; > - snprintf((char *) u->name, MAX_RAID_SERIAL_LEN, "%s", name); > + strncpy((char *) u->name, name, MAX_RAID_SERIAL_LEN); > + u->name[MAX_RAID_SERIAL_LEN-1] = '\0'; > append_metadata_update(st, u, sizeof(*u)); > } else { > struct imsm_dev *dev; > int i; > > dev = get_imsm_dev(super, vol); > - snprintf((char *) dev->volume, MAX_RAID_SERIAL_LEN, "%s", name); > + strncpy((char *) dev->volume, name, MAX_RAID_SERIAL_LEN); > + dev->volume[MAX_RAID_SERIAL_LEN-1] = '\0'; > for (i = 0; i < mpb->num_raid_devs; i++) { > dev = get_imsm_dev(super, i); > handle_missing(super, dev); These two look fine. Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation 2017-03-17 19:55 ` jes.sorensen @ 2017-03-18 1:02 ` Xiao Ni 0 siblings, 0 replies; 8+ messages in thread From: Xiao Ni @ 2017-03-18 1:02 UTC (permalink / raw) To: jes sorensen; +Cc: linux-raid, ncroxon, artur paszkiewicz ----- Original Message ----- > From: "jes sorensen" <jes.sorensen@gmail.com> > To: "Xiao Ni" <xni@redhat.com> > Cc: linux-raid@vger.kernel.org, ncroxon@redhat.com, "artur paszkiewicz" <artur.paszkiewicz@intel.com> > Sent: Saturday, March 18, 2017 3:55:43 AM > Subject: Re: [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation > > Xiao Ni <xni@redhat.com> writes: > > In gcc7 there are some building errors like: > > directive output may be truncated writing up to 31 bytes into a region of > > size 24 > > snprintf(str, MPB_SIG_LEN, %s, mpb->sig); > > > > It just need to copy one string to target. So use strncpy to replace it. > > > > Signed-off-by: Xiao Ni <xni@redhat.com> > > --- > > super-intel.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/super-intel.c b/super-intel.c > > index d5e9517..e1618f1 100644 > > --- a/super-intel.c > > +++ b/super-intel.c > > @@ -1811,7 +1811,8 @@ static void examine_super_imsm(struct supertype *st, > > char *homehost) > > __u32 reserved = imsm_reserved_sectors(super, super->disks); > > struct dl *dl; > > > > - snprintf(str, MPB_SIG_LEN, "%s", mpb->sig); > > + strncpy(str, (char *)mpb->sig, MPB_SIG_LEN); > > + str[MPB_SIG_LEN-1] = '\0'; > > printf(" Magic : %s\n", str); > > snprintf(str, strlen(MPB_VERSION_RAID0), "%s", get_imsm_version(mpb)); > > printf(" Version : %s\n", get_imsm_version(mpb)); > > Given str is defined as 'char str[MAX_SIGNATURE_LENGTH]', it would be > more correct to use MAX_SIGNATURE_LENGTH as the size limit here. I talked with Artur about this. It should use MPB_SIG_LEN, because the mpb->sig has version after the signature. It will print more if use MAX_SIGNATURE_LENGTH. > > > @@ -5227,7 +5228,7 @@ static int init_super_imsm_volume(struct supertype > > *st, mdu_array_info_t *info, > > disk->status = CONFIGURED_DISK | FAILED_DISK; > > disk->scsi_id = __cpu_to_le32(~(__u32)0); > > snprintf((char *) disk->serial, MAX_RAID_SERIAL_LEN, > > - "missing:%d", i); > > + "missing:%d", (__u8)i); > > } > > find_missing(super); > > } else { > > This is unrelated to the commit message. I'll re-send this patch later. Regards Xiao > > > @@ -7142,14 +7143,16 @@ static int update_subarray_imsm(struct supertype > > *st, char *subarray, > > > > u->type = update_rename_array; > > u->dev_idx = vol; > > - snprintf((char *) u->name, MAX_RAID_SERIAL_LEN, "%s", name); > > + strncpy((char *) u->name, name, MAX_RAID_SERIAL_LEN); > > + u->name[MAX_RAID_SERIAL_LEN-1] = '\0'; > > append_metadata_update(st, u, sizeof(*u)); > > } else { > > struct imsm_dev *dev; > > int i; > > > > dev = get_imsm_dev(super, vol); > > - snprintf((char *) dev->volume, MAX_RAID_SERIAL_LEN, "%s", name); > > + strncpy((char *) dev->volume, name, MAX_RAID_SERIAL_LEN); > > + dev->volume[MAX_RAID_SERIAL_LEN-1] = '\0'; > > for (i = 0; i < mpb->num_raid_devs; i++) { > > dev = get_imsm_dev(super, i); > > handle_missing(super, dev); > > These two look fine. > > Jes > -- > To unsubscribe from this list: send the line "unsubscribe linux-raid" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile 2017-03-17 11:55 [PATCH 0/3] mdadm: Fix some building errors Xiao Ni 2017-03-17 11:55 ` [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation Xiao Ni @ 2017-03-17 11:55 ` Xiao Ni 2017-03-17 19:57 ` jes.sorensen 2017-03-17 11:55 ` [PATCH 3/3] mdadm: Specify enough length when write to buffer Xiao Ni 2 siblings, 1 reply; 8+ messages in thread From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw) To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz There are many errors like 'error: this statement may fall through'. But the logic is right. So add the flag Wimplicit-fallthrough=0 to disable the error messages. The method I use is from https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html #index-Wimplicit-fallthrough-375 Signed-off-by: Xiao Ni <xni@redhat.com> --- Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index a6f464c..d1a6ac4 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,11 @@ ifdef WARN_UNUSED CWFLAGS += -Wp,-D_FORTIFY_SOURCE=2 -O3 endif +FALLTHROUGH := $(shell gcc -v --help 2>&1 | grep "implicit-fallthrough" | wc -l) +ifneq "$(FALLTHROUGH)" "0" +CWFLAGS += -Wimplicit-fallthrough=0 +endif + ifdef DEBIAN CPPFLAGS += -DDEBIAN endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile 2017-03-17 11:55 ` [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile Xiao Ni @ 2017-03-17 19:57 ` jes.sorensen 0 siblings, 0 replies; 8+ messages in thread From: jes.sorensen @ 2017-03-17 19:57 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz Xiao Ni <xni@redhat.com> writes: > There are many errors like 'error: this statement may fall through'. > But the logic is right. So add the flag Wimplicit-fallthrough=0 > to disable the error messages. The method I use is from > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html > #index-Wimplicit-fallthrough-375 > > Signed-off-by: Xiao Ni <xni@redhat.com> > --- > Makefile | 5 +++++ > 1 file changed, 5 insertions(+) Applied! Thanks, Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] mdadm: Specify enough length when write to buffer 2017-03-17 11:55 [PATCH 0/3] mdadm: Fix some building errors Xiao Ni 2017-03-17 11:55 ` [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation Xiao Ni 2017-03-17 11:55 ` [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile Xiao Ni @ 2017-03-17 11:55 ` Xiao Ni 2017-03-17 19:58 ` jes.sorensen 2 siblings, 1 reply; 8+ messages in thread From: Xiao Ni @ 2017-03-17 11:55 UTC (permalink / raw) To: linux-raid; +Cc: Jes.Sorensen, ncroxon, artur.paszkiewicz In Detail.c the buffer path in function Detail is defined as path[200], in fact the max lenth of content which needs to write to the buffer is 287. Because the length of dname of struct dirent is 255. During building it reports error: error: ‘%s’ directive writing up to 255 bytes into a region of size 189 [-Werror=format-overflow=] In function examine_super0 there is a buffer nb with length 5. But it need to show a int type argument. The lenght of max number of int is 10. So the buffer length should be 11. In human_size function the length of buf is 30. During building there is a error: output between 20 and 47 bytes into a destination of size 30. Change the length to 47. Signed-off-by: Xiao Ni <xni@redhat.com> --- Detail.c | 2 +- super0.c | 2 +- util.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Detail.c b/Detail.c index 509b0d4..cb33794 100644 --- a/Detail.c +++ b/Detail.c @@ -575,7 +575,7 @@ This is pretty boring printf(" Member Arrays :"); while (dir && (de = readdir(dir)) != NULL) { - char path[200]; + char path[287]; char vbuf[1024]; int nlen = strlen(sra->sys_name); dev_t devid; diff --git a/super0.c b/super0.c index 938cfd9..f5b4507 100644 --- a/super0.c +++ b/super0.c @@ -231,7 +231,7 @@ static void examine_super0(struct supertype *st, char *homehost) d++) { mdp_disk_t *dp; char *dv; - char nb[5]; + char nb[11]; int wonly, failfast; if (d>=0) dp = &sb->disks[d]; else dp = &sb->this_disk; diff --git a/util.c b/util.c index f100972..32bd909 100644 --- a/util.c +++ b/util.c @@ -811,7 +811,7 @@ unsigned long calc_csum(void *super, int bytes) #ifndef MDASSEMBLE char *human_size(long long bytes) { - static char buf[30]; + static char buf[47]; /* We convert bytes to either centi-M{ega,ibi}bytes or * centi-G{igi,ibi}bytes, with appropriate rounding, -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mdadm: Specify enough length when write to buffer 2017-03-17 11:55 ` [PATCH 3/3] mdadm: Specify enough length when write to buffer Xiao Ni @ 2017-03-17 19:58 ` jes.sorensen 0 siblings, 0 replies; 8+ messages in thread From: jes.sorensen @ 2017-03-17 19:58 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, ncroxon, artur.paszkiewicz Xiao Ni <xni@redhat.com> writes: > In Detail.c the buffer path in function Detail is defined as path[200], > in fact the max lenth of content which needs to write to the buffer is > 287. Because the length of dname of struct dirent is 255. > During building it reports error: > error: ‘%s’ directive writing up to 255 bytes into a region of size 189 > [-Werror=format-overflow=] > > In function examine_super0 there is a buffer nb with length 5. > But it need to show a int type argument. The lenght of max > number of int is 10. So the buffer length should be 11. > > In human_size function the length of buf is 30. During building > there is a error: > output between 20 and 47 bytes into a destination of size 30. > Change the length to 47. > > Signed-off-by: Xiao Ni <xni@redhat.com> > --- > Detail.c | 2 +- > super0.c | 2 +- > util.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) Applied! Thanks! Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-03-18 1:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-17 11:55 [PATCH 0/3] mdadm: Fix some building errors Xiao Ni 2017-03-17 11:55 ` [PATCH 1/3] mdadm: Replace snprintf with strncpy at some places to avoid truncation Xiao Ni 2017-03-17 19:55 ` jes.sorensen 2017-03-18 1:02 ` Xiao Ni 2017-03-17 11:55 ` [PATCH 2/3] mdadm: Add Wimplicit-fallthrough=0 in Makefile Xiao Ni 2017-03-17 19:57 ` jes.sorensen 2017-03-17 11:55 ` [PATCH 3/3] mdadm: Specify enough length when write to buffer Xiao Ni 2017-03-17 19:58 ` jes.sorensen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.