Re: svn commit: r322893 - head/bin/dd

[Bruce Evans]

On Fri, 25 Aug 2017, Conrad Meyer wrote:


On Fri, Aug 25, 2017 at 3:49 PM, Bruce Evans  wrote:

get_off_t() but not the higher level is fixed in my version.


Would you mind incorporating your version of dd into FreeBSD, or
publishing it so someone else can?


I guess it is in my queue, but unfortunately near the end.  My patches
for dd are about 8K (mostly style fixes).

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Conrad Meyer]

On Fri, Aug 25, 2017 at 3:49 PM, Bruce Evans  wrote:
> get_off_t() but not the higher level is fixed in my version.

Would you mind incorporating your version of dd into FreeBSD, or
publishing it so someone else can?

Thanks,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Bruce Evans]

On Fri, 25 Aug 2017, Matt Joras wrote:


On 08/25/2017 10:17, Conrad Meyer wrote:

This change seems to break buildworld on MIPS:

/home/cem/head.svn/bin/dd/args.c: In function 'f_bs':
/home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_cbs':
/home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_ibs':
/home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_obs':
/home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'

(Yes, it's odd that the SSIZE_MAX constant has 'long' type.)


SSIZE_MAX should have type long, since ssize_t is a long on mips (and
other arches besides i386 and arm).


Actually the reverse.  ssize_t has the correct type int32_t (which
happens to be int) on all 32-bit arches.  SSIZE_MAX shouldn't have
type long on 32-bit arches, but is broken by having that type on arm


Re: the build failure, that's in the GCC C format string checking, so
perhaps it's more accurate to say this breaks the (in-tree) GCC build.
%zd is the right format specifier for ssize_t. I guess GCC's format
string checking is getting confused because SSIZE_MAX is a constant that
expands to type long. Perhaps casting to ssize_t would GCC happier, but
that looks rather wrong.


This is because gcc's format checking actually works.  It detects that
SSIZE_MAX has the incorrect type long on mips because it is defined as
LONG_MAX there.  arm/arm64, powerpc and x86 have ifdefs to define it
correctly as INT_MAX in the 32-bit case.

I finally found where POSIX requires SSIZE_MAX to have the "correct" type
(POSIX doesn't define what that is. but C99 does).  SSIZE_MAX is just in
convered by the same rule as most C99 limits for integer types.

So SSIZE_MAX is not permitted to be what it is on mips.

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Bruce Evans]

On Fri, 25 Aug 2017, Conrad Meyer wrote:


Well, not negative, just large uint64_t numbers that would be negative
as off_t (int64_t).

E.g., dd if=/dev/kmem bs=1 iseek=0xf...foo count=8.  I think we
would like that to work.  I don't recall whether it does or not before
this change.


This is broken on 64-bit systems, first by the horrible get_off_t(),
then by broken range checking in at least old versions of dd.

The first bug in get_off_t() is that it uses strtoimax().  This fails
for offsets >= INTMAX_MAX.  This breaks the natural use of dd to seek
to the kernel part of kmem using dd.  E.g.:

dd if=/dev/kmem bs=1 count=1 iseek=0x802d2100

This should give a negative offset than then works.  (lseek() to negative
offsets is implementation-defined for special files, and FreeBSD defines
it so that it works for seeking in kmem.  This depends on some 2's complement
magic to represent large unsigned offsets as negative signed.)

However, it doesn't work to double bs and halve iseek.  The multiplication
is then done by a higher level.  It exceeds OFF_MAX, so the lseek() isn't
tried.

get_off_t() but not the higher level is fixed in my version.

It does work to calculate the negative offset in another way inside
get_off_t().  E.g., bs=1 iseek=-1 gives -1 which is passed to lseek().
bs=2 iseek=-1 also works to give the correct offset of -2 for lseek().
The arithmetic for converting large number like 0x802d2100 to
a negative value is painful, especially since shells have similar bugs
near INTMAX_MAX and UINTMAX_MAX and with signed and/or unsigned types
even if they support 64-bit integers.

The buggy range checking for bs=2 iseek=0x7fff802d2100 seems to be
only in old versions of dd.  It was in jcl(), and was just for the
initial args (multiplying them would exceed OFF_MAX).  Now there seems
to be no check for overflow.  The multiplication in pos_in() just
overflows even if the check in jcl() passed, later when the seek point
advances past INTMAX_MAX (if this exceeds OFF_MAX, then passing the
result of the multiplicating to lseek() starts overflowing before the
multiplication does).

[Context lost to top posting].

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Bruce Evans]

On Fri, 25 Aug 2017, Alan Somers wrote:


Log:
 dd(1): Incorrect casting of arguments


It is indeed now incorrect.


 dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and
 back again to detect whether or not the original arguments were negative.
 This is not correct, and causes problems with boundary cases, for example
 when count is SSIZE_MAX-1.


The casts were correct.  Did someone break the range checks?  I wrote most
of the cast, but would rarely write range checks using unportable casts.

SSIZE_MAX-1 isn't a boundary case.  The boundary is at SSIZE_MAX.  %zd
might work for it not.  But %zd is wrong for SSIZE_MAX-1, since the
expression might expand the type.


Modified: head/bin/dd/args.c
==
--- head/bin/dd/args.c  Fri Aug 25 14:42:11 2017(r322892)
+++ head/bin/dd/args.c  Fri Aug 25 15:31:55 2017(r322893)
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");

#include 

+#include 


Unrelated change.


#include 
#include 
#include 
@@ -184,7 +185,7 @@ f_bs(char *arg)

res = get_num(arg);
if (res < 1 || res > SSIZE_MAX)
-   errx(1, "bs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
+   errx(1, "bs must be between 1 and %zd", SSIZE_MAX);
in.dbsz = out.dbsz = (size_t)res;
}



This is incorrect.  The old version used the portable method of casting
a signed value of unknown type to intmax_t before %zd existed.  %zd still
isn't useable in either theory or practice for printing ssize_t's.

Theory:
C99 defines %zd as being for the signed type "corresponding" to size_t.
Who knows what "corresponding" is except for pure 2's complement with
no padding bits?  POSIX only defines ssize_t circularly as being "any
signed integer type capable of representing the range -1 through
SSIZE_MAX" where SSIZE_MAX is the maximum of ssize_t".  It has a few
restrictions that make this not completely circular, but I couldn't
find anywhere where it states the usual restriction on limits -- that
they have the type of the default promotion of the type that they limit.

Practice:
As reported in other replies, some arches define SSIZE_MAX in a way that
makes its type not the default promotion of ssize_t.  Most likely
SSIZE_MAX is bogusly long and ssize_t is int, or vice versa, on an
arch where int and long have the same representation.  Printf format
checking reports this type mismatch because it would be fatal on other
arches.


@@ -195,22 +196,22 @@ f_cbs(char *arg)

res = get_num(arg);
if (res < 1 || res > SSIZE_MAX)
-   errx(1, "cbs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
+   errx(1, "cbs must be between 1 and %zd", SSIZE_MAX);
cbsz = (size_t)res;
}


Similarly.



static void
f_count(char *arg)
{
-   intmax_t res;
+   uintmax_t res;


This breaks the not so careful checking for negative args.

dd wants to allow negative args for seek offsets and not much else.
It has a badly written get_off_t() to handle the offsets, but old code
like this still used the old function get_num() that returns an unsigned
type.  This is a feature since get_off_t() is so badly written that it
is better to convert get_num() almost as done here.  My version does this
internally:

X /*
X  * Convert an expression of the above forms to an off_t.  This version does
X  * not handle negative numbers perfectly.  It assumes 2's complement and
X  * maybe nonnegative multipliers.  I hope perfect handling is not necessary
X  * for dd.
X  */
X static off_t
X get_off_t(const char *val)
X {
X   u_quad_t num;
X 
X 	num = get_num(val);

X   if (num != (u_quad_t)(off_t)num)
X   errx(1, "%s: offset too large", oper);
X   return ((off_t)num);
X }

This still uses the bogus type u_quad_t since that is what get_num() returns
in the old version of dd that this patch is for.

There should be no restriction except UINTMAX_MAX on counts.  This is fixed
in my version:

X static void
X f_count(char *arg)
X {
X 
X 	cpy_cnt = get_num(arg);

X #if 0
X   if (cpy_cnt == 0)
X   terminate(0);
X #endif
X }

get_num() already did correct range checking.

A count of 0 should mean infinity.

Code elsewhere needs be careful to go to infinity for count 0 and to
not multiply a large count by a block size.  In practice, counts of
nearly UINTMAX_MAX are unreachable.



-   res = (intmax_t)get_num(arg);
-   if (res < 0)
-   errx(1, "count cannot be negative");
+   res = get_num(arg);
+   if (res == UINTMAX_MAX)
+   errc(1, ERANGE, "%s", oper);


This is nonsense error checking.  get_num() already did correct checking
and exited on range errors.  A value of UINTMAX_MAX is not necessarily an
error.  After calling strtoumax(), it might be either a range error or at
the end of the range (including the negative end).  Since get_num() returned,
it was not a range error.  The check here does extra work to 

Re: svn commit: r322893 - head/bin/dd

[Ian Lepore]

On Fri, 2017-08-25 at 12:09 -0600, Warner Losh wrote:
> On Fri, Aug 25, 2017 at 12:02 PM, Matt Joras 
> wrote:
> 
> > 
> > On 08/25/2017 10:17, Conrad Meyer wrote:
> > > 
> > > This change seems to break buildworld on MIPS:
> > > 
> > > /home/cem/head.svn/bin/dd/args.c: In function 'f_bs':
> > > /home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd'
> > > expects
> > > type 'signed size_t', but argument 3 has type 'long int'
> > > /home/cem/head.svn/bin/dd/args.c: In function 'f_cbs':
> > > /home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd'
> > > expects
> > > type 'signed size_t', but argument 3 has type 'long int'
> > > /home/cem/head.svn/bin/dd/args.c: In function 'f_ibs':
> > > /home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd'
> > > expects
> > > type 'signed size_t', but argument 3 has type 'long int'
> > > /home/cem/head.svn/bin/dd/args.c: In function 'f_obs':
> > > /home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd'
> > > expects
> > > type 'signed size_t', but argument 3 has type 'long int'
> > > 
> > > (Yes, it's odd that the SSIZE_MAX constant has 'long' type.)
> > > 
> > SSIZE_MAX should have type long, since ssize_t is a long on mips
> > (and
> > other arches besides i386 and arm).
> > 
> > Re: the build failure, that's in the GCC C format string checking,
> > so
> > perhaps it's more accurate to say this breaks the (in-tree) GCC
> > build.
> > %zd is the right format specifier for ssize_t. I guess GCC's format
> > string checking is getting confused because SSIZE_MAX is a constant
> > that
> > expands to type long. Perhaps casting to ssize_t would GCC happier,
> > but
> > that looks rather wrong.
> > 
> This is why it was cast in the first place due to issues with exact
> type.
> Maybe we should put the casts back for the printfs.
> 
> Warner

I think the right fix is to define SSIZE_MAX correctly based on the
type of ssize_t.  The x86 and powerpc _limits.h files get this right,
it looks like mips is the only one with both 32 and 64-bit support that
doesn't define SSIZE_MAX based on ssize_t type.

-- Ian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Alan Somers]

That doesn't work either before or after this change.
-Alan

On Fri, Aug 25, 2017 at 11:20 AM, Conrad Meyer  wrote:
> Well, not negative, just large uint64_t numbers that would be negative
> as off_t (int64_t).
>
> E.g., dd if=/dev/kmem bs=1 iseek=0xf...foo count=8.  I think we
> would like that to work.  I don't recall whether it does or not before
> this change.
>
> Best,
> Conrad
>
> On Fri, Aug 25, 2017 at 10:08 AM, Alan Somers  wrote:
>> Nope.  Do you mean negative offsets for the iseek argument?  I didn't
>> know you could do that.
>>
>> On Fri, Aug 25, 2017 at 10:59 AM, Conrad Meyer  wrote:
>>> Hi Alan,
>>>
>>> By any chance did you test this change with /dev/kmem and kernel
>>> addresses ("negative" off_t values)?
>>>
>>> Thanks,
>>> Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Warner Losh]

On Fri, Aug 25, 2017 at 12:02 PM, Matt Joras  wrote:

> On 08/25/2017 10:17, Conrad Meyer wrote:
> > This change seems to break buildworld on MIPS:
> >
> > /home/cem/head.svn/bin/dd/args.c: In function 'f_bs':
> > /home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd' expects
> > type 'signed size_t', but argument 3 has type 'long int'
> > /home/cem/head.svn/bin/dd/args.c: In function 'f_cbs':
> > /home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd' expects
> > type 'signed size_t', but argument 3 has type 'long int'
> > /home/cem/head.svn/bin/dd/args.c: In function 'f_ibs':
> > /home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd' expects
> > type 'signed size_t', but argument 3 has type 'long int'
> > /home/cem/head.svn/bin/dd/args.c: In function 'f_obs':
> > /home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd' expects
> > type 'signed size_t', but argument 3 has type 'long int'
> >
> > (Yes, it's odd that the SSIZE_MAX constant has 'long' type.)
> >
> SSIZE_MAX should have type long, since ssize_t is a long on mips (and
> other arches besides i386 and arm).
>
> Re: the build failure, that's in the GCC C format string checking, so
> perhaps it's more accurate to say this breaks the (in-tree) GCC build.
> %zd is the right format specifier for ssize_t. I guess GCC's format
> string checking is getting confused because SSIZE_MAX is a constant that
> expands to type long. Perhaps casting to ssize_t would GCC happier, but
> that looks rather wrong.
>

This is why it was cast in the first place due to issues with exact type.
Maybe we should put the casts back for the printfs.

Warner
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Matt Joras]

On 08/25/2017 10:17, Conrad Meyer wrote:
> This change seems to break buildworld on MIPS:
>
> /home/cem/head.svn/bin/dd/args.c: In function 'f_bs':
> /home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd' expects
> type 'signed size_t', but argument 3 has type 'long int'
> /home/cem/head.svn/bin/dd/args.c: In function 'f_cbs':
> /home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd' expects
> type 'signed size_t', but argument 3 has type 'long int'
> /home/cem/head.svn/bin/dd/args.c: In function 'f_ibs':
> /home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd' expects
> type 'signed size_t', but argument 3 has type 'long int'
> /home/cem/head.svn/bin/dd/args.c: In function 'f_obs':
> /home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd' expects
> type 'signed size_t', but argument 3 has type 'long int'
>
> (Yes, it's odd that the SSIZE_MAX constant has 'long' type.)
>
SSIZE_MAX should have type long, since ssize_t is a long on mips (and
other arches besides i386 and arm).

Re: the build failure, that's in the GCC C format string checking, so
perhaps it's more accurate to say this breaks the (in-tree) GCC build.
%zd is the right format specifier for ssize_t. I guess GCC's format
string checking is getting confused because SSIZE_MAX is a constant that
expands to type long. Perhaps casting to ssize_t would GCC happier, but
that looks rather wrong.

Matt
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Conrad Meyer]

Well, not negative, just large uint64_t numbers that would be negative
as off_t (int64_t).

E.g., dd if=/dev/kmem bs=1 iseek=0xf...foo count=8.  I think we
would like that to work.  I don't recall whether it does or not before
this change.

Best,
Conrad

On Fri, Aug 25, 2017 at 10:08 AM, Alan Somers  wrote:
> Nope.  Do you mean negative offsets for the iseek argument?  I didn't
> know you could do that.
>
> On Fri, Aug 25, 2017 at 10:59 AM, Conrad Meyer  wrote:
>> Hi Alan,
>>
>> By any chance did you test this change with /dev/kmem and kernel
>> addresses ("negative" off_t values)?
>>
>> Thanks,
>> Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Conrad Meyer]

This change seems to break buildworld on MIPS:

/home/cem/head.svn/bin/dd/args.c: In function 'f_bs':
/home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_cbs':
/home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_ibs':
/home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'
/home/cem/head.svn/bin/dd/args.c: In function 'f_obs':
/home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd' expects
type 'signed size_t', but argument 3 has type 'long int'

(Yes, it's odd that the SSIZE_MAX constant has 'long' type.)

Best,
Conrad

On Fri, Aug 25, 2017 at 8:31 AM, Alan Somers  wrote:
> Author: asomers
> Date: Fri Aug 25 15:31:55 2017
> New Revision: 322893
> URL: https://svnweb.freebsd.org/changeset/base/322893
>
> Log:
>   dd(1): Incorrect casting of arguments
>
>   dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and
>   back again to detect whether or not the original arguments were negative.
>   This is not correct, and causes problems with boundary cases, for example
>   when count is SSIZE_MAX-1.
>
>   PR:   191263
>   Submitted by: w...@worrbase.com
>   Reviewed by:  pi, asomers
>   MFC after:3 weeks
>
> Modified:
>   head/bin/dd/args.c
>   head/bin/dd/conv.c
>   head/bin/dd/dd.c
>   head/bin/dd/dd.h
>   head/bin/dd/position.c
>
> Modified: head/bin/dd/args.c
> ==
> --- head/bin/dd/args.c  Fri Aug 25 14:42:11 2017(r322892)
> +++ head/bin/dd/args.c  Fri Aug 25 15:31:55 2017(r322893)
> @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
>
>  #include 
>
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -184,7 +185,7 @@ f_bs(char *arg)
>
> res = get_num(arg);
> if (res < 1 || res > SSIZE_MAX)
> -   errx(1, "bs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
> +   errx(1, "bs must be between 1 and %zd", SSIZE_MAX);
> in.dbsz = out.dbsz = (size_t)res;
>  }
>
> @@ -195,22 +196,22 @@ f_cbs(char *arg)
>
> res = get_num(arg);
> if (res < 1 || res > SSIZE_MAX)
> -   errx(1, "cbs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
> +   errx(1, "cbs must be between 1 and %zd", SSIZE_MAX);
> cbsz = (size_t)res;
>  }
>
>  static void
>  f_count(char *arg)
>  {
> -   intmax_t res;
> +   uintmax_t res;
>
> -   res = (intmax_t)get_num(arg);
> -   if (res < 0)
> -   errx(1, "count cannot be negative");
> +   res = get_num(arg);
> +   if (res == UINTMAX_MAX)
> +   errc(1, ERANGE, "%s", oper);
> if (res == 0)
> -   cpy_cnt = (uintmax_t)-1;
> +   cpy_cnt = UINTMAX_MAX;
> else
> -   cpy_cnt = (uintmax_t)res;
> +   cpy_cnt = res;
>  }
>
>  static void
> @@ -219,7 +220,7 @@ f_files(char *arg)
>
> files_cnt = get_num(arg);
> if (files_cnt < 1)
> -   errx(1, "files must be between 1 and %jd", (uintmax_t)-1);
> +   errx(1, "files must be between 1 and %zu", SIZE_MAX);
>  }
>
>  static void
> @@ -240,8 +241,8 @@ f_ibs(char *arg)
> if (!(ddflags & C_BS)) {
> res = get_num(arg);
> if (res < 1 || res > SSIZE_MAX)
> -   errx(1, "ibs must be between 1 and %jd",
> -   (intmax_t)SSIZE_MAX);
> +   errx(1, "ibs must be between 1 and %zd",
> +   SSIZE_MAX);
> in.dbsz = (size_t)res;
> }
>  }
> @@ -261,8 +262,8 @@ f_obs(char *arg)
> if (!(ddflags & C_BS)) {
> res = get_num(arg);
> if (res < 1 || res > SSIZE_MAX)
> -   errx(1, "obs must be between 1 and %jd",
> -   (intmax_t)SSIZE_MAX);
> +   errx(1, "obs must be between 1 and %zd",
> +   SSIZE_MAX);
> out.dbsz = (size_t)res;
> }
>  }
>
> Modified: head/bin/dd/conv.c
> ==
> --- head/bin/dd/conv.c  Fri Aug 25 14:42:11 2017(r322892)
> +++ head/bin/dd/conv.c  Fri Aug 25 15:31:55 2017(r322893)
> @@ -133,7 +133,7 @@ block(void)
>  */
> ch = 0;
> for (inp = in.dbp - in.dbcnt, outp = out.dbp; in.dbcnt;) {
> -   maxlen = MIN(cbsz, in.dbcnt);
> +   maxlen = MIN(cbsz, (size_t)in.dbcnt);
> if ((t = ctab) != NULL)
> for (cnt = 0; cnt < maxlen && (ch = *inp++) != '\n';
> ++cnt)
> @@ -146,7 +146,7 @@ block(void)
> 

Re: svn commit: r322893 - head/bin/dd

[Alan Somers]

Nope.  Do you mean negative offsets for the iseek argument?  I didn't
know you could do that.

On Fri, Aug 25, 2017 at 10:59 AM, Conrad Meyer  wrote:
> Hi Alan,
>
> By any chance did you test this change with /dev/kmem and kernel
> addresses ("negative" off_t values)?
>
> Thanks,
> Conrad
>
> On Fri, Aug 25, 2017 at 8:31 AM, Alan Somers  wrote:
>> Author: asomers
>> Date: Fri Aug 25 15:31:55 2017
>> New Revision: 322893
>> URL: https://svnweb.freebsd.org/changeset/base/322893
>>
>> Log:
>>   dd(1): Incorrect casting of arguments
>>
>>   dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and
>>   back again to detect whether or not the original arguments were negative.
>>   This is not correct, and causes problems with boundary cases, for example
>>   when count is SSIZE_MAX-1.
>>
>>   PR:   191263
>>   Submitted by: w...@worrbase.com
>>   Reviewed by:  pi, asomers
>>   MFC after:3 weeks
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322893 - head/bin/dd

[Conrad Meyer]

Hi Alan,

By any chance did you test this change with /dev/kmem and kernel
addresses ("negative" off_t values)?

Thanks,
Conrad

On Fri, Aug 25, 2017 at 8:31 AM, Alan Somers  wrote:
> Author: asomers
> Date: Fri Aug 25 15:31:55 2017
> New Revision: 322893
> URL: https://svnweb.freebsd.org/changeset/base/322893
>
> Log:
>   dd(1): Incorrect casting of arguments
>
>   dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and
>   back again to detect whether or not the original arguments were negative.
>   This is not correct, and causes problems with boundary cases, for example
>   when count is SSIZE_MAX-1.
>
>   PR:   191263
>   Submitted by: w...@worrbase.com
>   Reviewed by:  pi, asomers
>   MFC after:3 weeks
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"