Message ID | 20171205235651.17102-2-ross.zwisler@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote: > I recently upgraded my compiler from > gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) > to > gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) > and started getting a bunch of compiler warnings in io/fsmap.c: > > fsmap.c: In function ‘fsmap_f’: > fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing > between 1 and 17 bytes into a region of size between 12 and 28 > [-Wformat-truncation=] > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~ > fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~~~~~~~~~~~~ > fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a > destination of size 32 > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > (long long)BTOBBT(p->fmr_physical), > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each > signed long long can potentially print as many as 19 characters > (9223372036854775807 is the max value). The format we're using for bbuf is > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters > "[..]:\0", which means it's possible we'll print up to 44 characters, > overflowing our 32 char buffer. > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 > characters. > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > Cc: Darrick J. Wong <darrick.wong@oracle.com> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") > --- > io/fsmap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/io/fsmap.c b/io/fsmap.c > index 448fb535..3d8a6700 100644 > --- a/io/fsmap.c > +++ b/io/fsmap.c > @@ -184,8 +184,8 @@ dump_map_verbose( > off64_t agoff, bperag; > int foff_w, boff_w, aoff_w, tot_w, agno_w, own_w; > int nr_w, dev_w; > - char rbuf[32], bbuf[32], abuf[32], obuf[32]; > - char nbuf[32], dbuf[32], gbuf[32]; > + char rbuf[64], bbuf[64], abuf[64], obuf[64]; > + char nbuf[64], dbuf[64], gbuf[64]; > char owner[OWNER_BUF_SZ]; > int sunit, swidth; > int flg = 0; > -- > 2.14.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote: > I recently upgraded my compiler from > gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) > to > gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) > and started getting a bunch of compiler warnings in io/fsmap.c: > > fsmap.c: In function ‘fsmap_f’: > fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing > between 1 and 17 bytes into a region of size between 12 and 28 > [-Wformat-truncation=] > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~ > fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~~~~~~~~~~~~ > fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a > destination of size 32 > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > (long long)BTOBBT(p->fmr_physical), > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each > signed long long can potentially print as many as 19 characters > (9223372036854775807 is the max value). The format we're using for bbuf is > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters > "[..]:\0", which means it's possible we'll print up to 44 characters, > overflowing our 32 char buffer. > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 > characters. > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > Cc: Darrick J. Wong <darrick.wong@oracle.com> > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") FYI, I posted a fix for this weeks ago. I think Eric has already picked it up, but it hasn't been pushed out into the for-next branch yet. Cheers, Dave.
On 12/5/17 6:27 PM, Dave Chinner wrote: > FYI, I posted a fix for this weeks ago. I think Eric has already > picked it up, but it hasn't been pushed out into the for-next branch > yet. Yes, sorry for the delay. Will be pushed out soon. > Cheers, > > Dave. > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote: > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote: > > I recently upgraded my compiler from > > gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) > > to > > gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) > > and started getting a bunch of compiler warnings in io/fsmap.c: > > > > fsmap.c: In function ‘fsmap_f’: > > fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing > > between 1 and 17 bytes into a region of size between 12 and 28 > > [-Wformat-truncation=] > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > ^~~~ > > fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > ^~~~~~~~~~~~~~~ > > fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a > > destination of size 32 > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > (long long)BTOBBT(p->fmr_physical), > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each > > signed long long can potentially print as many as 19 characters > > (9223372036854775807 is the max value). The format we're using for bbuf is > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters > > "[..]:\0", which means it's possible we'll print up to 44 characters, > > overflowing our 32 char buffer. > > > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 > > characters. > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > Cc: Darrick J. Wong <darrick.wong@oracle.com> > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") > > FYI, I posted a fix for this weeks ago. I think Eric has already > picked it up, but it hasn't been pushed out into the for-next branch > yet. I'm seeing similar new compiler warnings when compiling xfstests: write_log.c: In function ‘wlog_open’: write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=] "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", ^~ write_log.c:124:4: note: directive argument in the range [1089, 2047] "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ etc. I don't see any patches posted that fix these, as of yet. As far as you know, am I correct in thinking that these still need to be fixed? - Ross -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote: > On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote: > > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote: > > > I recently upgraded my compiler from > > > gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) > > > to > > > gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) > > > and started getting a bunch of compiler warnings in io/fsmap.c: > > > > > > fsmap.c: In function ‘fsmap_f’: > > > fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing > > > between 1 and 17 bytes into a region of size between 12 and 28 > > > [-Wformat-truncation=] > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > ^~~~ > > > fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > ^~~~~~~~~~~~~~~ > > > fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a > > > destination of size 32 > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > (long long)BTOBBT(p->fmr_physical), > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each > > > signed long long can potentially print as many as 19 characters > > > (9223372036854775807 is the max value). The format we're using for bbuf is > > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters > > > "[..]:\0", which means it's possible we'll print up to 44 characters, > > > overflowing our 32 char buffer. > > > > > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 > > > characters. > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > Cc: Darrick J. Wong <darrick.wong@oracle.com> > > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") > > > > FYI, I posted a fix for this weeks ago. I think Eric has already > > picked it up, but it hasn't been pushed out into the for-next branch > > yet. > > I'm seeing similar new compiler warnings when compiling xfstests: > > write_log.c: In function ‘wlog_open’: > write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=] > "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", > ^~ > write_log.c:124:4: note: directive argument in the range [1089, 2047] > "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > etc. > > I don't see any patches posted that fix these, as of yet. As far as you know, > am I correct in thinking that these still need to be fixed? It sure looks that way. --D > - Ross > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 06, 2017 at 12:47:49PM -0800, Darrick J. Wong wrote: > On Wed, Dec 06, 2017 at 01:10:14PM -0700, Ross Zwisler wrote: > > On Wed, Dec 06, 2017 at 11:27:43AM +1100, Dave Chinner wrote: > > > On Tue, Dec 05, 2017 at 04:56:49PM -0700, Ross Zwisler wrote: > > > > I recently upgraded my compiler from > > > > gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) > > > > to > > > > gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) > > > > and started getting a bunch of compiler warnings in io/fsmap.c: > > > > > > > > fsmap.c: In function ‘fsmap_f’: > > > > fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing > > > > between 1 and 17 bytes into a region of size between 12 and 28 > > > > [-Wformat-truncation=] > > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > > ^~~~ > > > > fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] > > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > > ^~~~~~~~~~~~~~~ > > > > fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a > > > > destination of size 32 > > > > snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > (long long)BTOBBT(p->fmr_physical), > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > > > The issue is that 'bbuf' is only defined to be 32 characters wide, but each > > > > signed long long can potentially print as many as 19 characters > > > > (9223372036854775807 is the max value). The format we're using for bbuf is > > > > "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters > > > > "[..]:\0", which means it's possible we'll print up to 44 characters, > > > > overflowing our 32 char buffer. > > > > > > > > Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 > > > > characters. > > > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > > Cc: Darrick J. Wong <darrick.wong@oracle.com> > > > > Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") > > > > > > FYI, I posted a fix for this weeks ago. I think Eric has already > > > picked it up, but it hasn't been pushed out into the for-next branch > > > yet. > > > > I'm seeing similar new compiler warnings when compiling xfstests: > > > > write_log.c: In function ‘wlog_open’: > > write_log.c:124:37: warning: ‘%s’ directive writing up to 1023 bytes into a region of size 224 [-Wformat-overflow=] > > "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", > > ^~ > > write_log.c:124:4: note: directive argument in the range [1089, 2047] > > "Could not open write_log - open(%s, %#o, %#o) failed: %s\n", > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > etc. > > > > I don't see any patches posted that fix these, as of yet. As far as you know, > > am I correct in thinking that these still need to be fixed? > > It sure looks that way. Cool, thanks. I'll take a crack at them. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/io/fsmap.c b/io/fsmap.c index 448fb535..3d8a6700 100644 --- a/io/fsmap.c +++ b/io/fsmap.c @@ -184,8 +184,8 @@ dump_map_verbose( off64_t agoff, bperag; int foff_w, boff_w, aoff_w, tot_w, agno_w, own_w; int nr_w, dev_w; - char rbuf[32], bbuf[32], abuf[32], obuf[32]; - char nbuf[32], dbuf[32], gbuf[32]; + char rbuf[64], bbuf[64], abuf[64], obuf[64]; + char nbuf[64], dbuf[64], gbuf[64]; char owner[OWNER_BUF_SZ]; int sunit, swidth; int flg = 0;
I recently upgraded my compiler from gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1) to gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) and started getting a bunch of compiler warnings in io/fsmap.c: fsmap.c: In function ‘fsmap_f’: fsmap.c:228:40: warning: ‘%lld’ directive output may be truncated writing between 1 and 17 bytes into a region of size between 12 and 28 [-Wformat-truncation=] snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", ^~~~ fsmap.c:228:32: note: directive argument in the range [0, 36028797018963967] snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", ^~~~~~~~~~~~~~~ fsmap.c:228:3: note: ‘snprintf’ output between 8 and 40 bytes into a destination of size 32 snprintf(bbuf, sizeof(bbuf), "[%lld..%lld]:", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ (long long)BTOBBT(p->fmr_physical), ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ (long long)BTOBBT(p->fmr_physical + p->fmr_length - 1)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The issue is that 'bbuf' is only defined to be 32 characters wide, but each signed long long can potentially print as many as 19 characters (9223372036854775807 is the max value). The format we're using for bbuf is "[%lld..%lld]:" which has 2 signed long longs plus 6 other characters "[..]:\0", which means it's possible we'll print up to 44 characters, overflowing our 32 char buffer. Fix this by bumping all the buffer sizes in dump_map_verbose() to 64 characters. Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> Cc: Darrick J. Wong <darrick.wong@oracle.com> Fixes: 3fcab549a234 ("xfs_io: support the new getfsmap ioctl") --- io/fsmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)