Re: Bug in dd's args.c, invalid check for error

[Philip Guenther]

On Tue, Jan 2, 2018 at 3:06 PM, Ingo Schwarze  wrote:
...

> OK for the patch at the end?
>
> Testing - before:
>
...

> --- args.c  16 Aug 2016 16:44:55 -  1.28
> +++ args.c  2 Jan 2018 22:47:19 -
> @@ -406,8 +406,9 @@ get_off(char *val)
> off_t num, t;
> char *expr;
>
> +   errno = 0;
> num = strtoll(val, , 0);
> -   if (num == LLONG_MAX)   /* Overflow. */
> +   if (num == LLONG_MAX && errno == ERANGE)/* Overflow. */
> err(1, "%s", oper);
> if (expr == val)/* No digits. */
> errx(1, "%s: illegal numeric value", oper);
>

ok guenther@

Re: Bug in dd's args.c, invalid check for error

[Ingo Schwarze]

Hi,

Bulat Musin wrote on Tue, Jan 02, 2018 at 09:47:39PM +0300:

> /bin/dd/args.c r1.28
> get_off(char *val)
> 
>   num = strtoll(val, , 0);
>   if (num == LLONG_MAX)   /* Overflow. */
>   err(1, "%s", oper);
> 
> Incorrect checking of overflow.
> Firstly, set errno to 0 before calling strtoll.
> Secondly: check of errno == ERANGE. =>

Clearly an edge case, but i think you are right that it ought to
be fixed, if only to avoid the bad example.  I also checked that
clobbering the previous value of errno is not a problem here.

OK for the patch at the end?

Testing - before:

 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7ffe
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (116877 bytes/sec)
 $ echo $?
0
 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7fff
dd: skip: Undefined error: 0/* wrong */
 $ echo $?
1   /* wrong */
 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x8000
dd: skip: Result too large
 $ echo $?
1

Testing - after:

 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7ffe
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (230920 bytes/sec)
 $ echo $?
0
 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7fff
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (78453 bytes/sec)
 $ echo $?
0
 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x8000
dd: skip: Result too large
 $ echo $?
1

Yours,
  Ingo


Index: args.c
===
RCS file: /cvs/src/bin/dd/args.c,v
retrieving revision 1.28
diff -u -p -r1.28 args.c
--- args.c  16 Aug 2016 16:44:55 -  1.28
+++ args.c  2 Jan 2018 22:47:19 -
@@ -406,8 +406,9 @@ get_off(char *val)
off_t num, t;
char *expr;
 
+   errno = 0;
num = strtoll(val, , 0);
-   if (num == LLONG_MAX)   /* Overflow. */
+   if (num == LLONG_MAX && errno == ERANGE)/* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);

Bug in dd's args.c, invalid check for error

[Bulat Musin]

/bin/dd/args.c r1.28

get_off(char *val)

Code:

num = strtoll(val, , 0);
if (num == LLONG_MAX)   /* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);

Incorrect checking of overflow.
Firstly, set errno to 0 before calling strtoll.
Secondly: check of errno == ERANGE. =>

errno = 0;
num = strtoll(val, , 0);
if (errno == ERANGE && num == LLONG_MAX)/* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);


https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/dd/args.c?rev=1.28