diff mbox

[xfsprogs,v2,1/3] xfs_io: fix compiler warnings in getfsmap code

Message ID 20171205235651.17102-2-ross.zwisler@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ross Zwisler Dec. 5, 2017, 11:56 p.m. UTC
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(-)

Comments

Darrick J. Wong Dec. 6, 2017, 12:03 a.m. UTC | #1
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
Dave Chinner Dec. 6, 2017, 12:27 a.m. UTC | #2
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.
Eric Sandeen Dec. 6, 2017, 1:59 p.m. UTC | #3
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
Ross Zwisler Dec. 6, 2017, 8:10 p.m. UTC | #4
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
Darrick J. Wong Dec. 6, 2017, 8:47 p.m. UTC | #5
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
Ross Zwisler Dec. 6, 2017, 8:58 p.m. UTC | #6
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 mbox

Patch

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;