Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-05-14 Thread Christos Zoulas
In article <95034282-ebf6-c1d5-8bb1-9258ee825...@marples.name>,
Roy Marples   wrote:
>On 10/05/2020 18:58, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Sun May 10 17:58:16 UTC 2020
>> 
>> Modified Files:
>>  src/external/bsd/dhcpcd/dist/src: dhcpcd.c
>> 
>> Log Message:
>> Add SIGPIPE to the list of dhcpcd affected signals since we sigignore it.
>
>Why?

Because the forked programs from scripts were executed with SIGPIPE blocked.
If that breaks non-kqueue OS's we should just add SIGPIPE to the signal
mask to default for posix_spawn().

christos



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-05-14 Thread Roy Marples

On 10/05/2020 18:58, Christos Zoulas wrote:

Module Name:src
Committed By:   christos
Date:   Sun May 10 17:58:16 UTC 2020

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcpcd.c

Log Message:
Add SIGPIPE to the list of dhcpcd affected signals since we sigignore it.


Why?

BTW, this breaks non kqueue OS's so can't be accepted upstream.

Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-15 Thread Roy Marples

On 14/02/2020 20:36, Santhosh Raju wrote:

On Thu, Feb 13, 2020 at 4:44 PM Santhosh Raju  wrote:


On Thu, Feb 13, 2020 at 4:32 PM Kamil Rytarowski  wrote:


On 13.02.2020 22:20, Valery Ushakov wrote:

I did not propose to disable the warning.  I proposed to downgrade
-Werror to -Wno-error (i.e. a warning) and only for the buggy
sanitizer build.  That file will still be compiled in normal builds
with all the warnings=errors enabled, so real problems won't be
overlooked.


OK, we can try this path.

Santosh, could you please revert and try -Wno-error + upstream it?

Thank you in advance!



Sure, let me prepare the patch.



The patch has been prepared.

The builds were run both with and without MKLIBCSANITIZER=yes and it
was completed successfully.

Let us know if it alright to commit this.


Looks ok to me, commit it.

Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-14 Thread Santhosh Raju
On Thu, Feb 13, 2020 at 4:44 PM Santhosh Raju  wrote:
>
> On Thu, Feb 13, 2020 at 4:32 PM Kamil Rytarowski  wrote:
> >
> > On 13.02.2020 22:20, Valery Ushakov wrote:
> > > I did not propose to disable the warning.  I proposed to downgrade
> > > -Werror to -Wno-error (i.e. a warning) and only for the buggy
> > > sanitizer build.  That file will still be compiled in normal builds
> > > with all the warnings=errors enabled, so real problems won't be
> > > overlooked.
> >
> > OK, we can try this path.
> >
> > Santosh, could you please revert and try -Wno-error + upstream it?
> >
> > Thank you in advance!
> >
>
> Sure, let me prepare the patch.
>

The patch has been prepared.

The builds were run both with and without MKLIBCSANITIZER=yes and it
was completed successfully.

Let us know if it alright to commit this.

> --
> Santhosh

--
Santhosh
Index: dist/src/dhcp.c
===
RCS file: /cvsroot/src/external/bsd/dhcpcd/dist/src/dhcp.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 dhcp.c
--- dist/src/dhcp.c	8 Feb 2020 12:17:16 -	1.33
+++ dist/src/dhcp.c	14 Feb 2020 20:32:20 -
@@ -3307,7 +3307,7 @@ is_packet_udp_bootp(void *packet, size_t
 	memcpy(, (char *)ip + ip_hlen, sizeof(udp));
 	if (ntohs(udp.uh_ulen) < sizeof(udp))
 		return false;
-	if (ip_hlen + (size_t)ntohs(udp.uh_ulen) > plen)
+	if (ip_hlen + ntohs(udp.uh_ulen) > plen)
 		return false;
 
 	/* Check it's to and from the right ports. */
Index: sbin/dhcpcd/Makefile
===
RCS file: /cvsroot/src/external/bsd/dhcpcd/sbin/dhcpcd/Makefile,v
retrieving revision 1.50
diff -u -p -u -r1.50 Makefile
--- sbin/dhcpcd/Makefile	29 Jan 2020 23:42:57 -	1.50
+++ sbin/dhcpcd/Makefile	14 Feb 2020 20:32:20 -
@@ -27,6 +27,11 @@ SRCS+=		auth.c
 .if (${USE_INET} != "no")
 CPPFLAGS+=	-DINET
 SRCS+=		bpf.c dhcp.c ipv4.c
+.if (${MKLIBCSANITIZER:Uno} == "yes")
+.if (${ACTIVE_CC} == "gcc" && ${HAVE_GCC:U0} == 8)
+COPTS.dhcp.c+=	-Wno-error=sign-conversion
+.endif
+.endif
 .if !defined(SMALLPROG)
 CPPFLAGS+=	-DARP
 SRCS+=		arp.c


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Santhosh Raju
On Thu, Feb 13, 2020 at 4:32 PM Kamil Rytarowski  wrote:
>
> On 13.02.2020 22:20, Valery Ushakov wrote:
> > I did not propose to disable the warning.  I proposed to downgrade
> > -Werror to -Wno-error (i.e. a warning) and only for the buggy
> > sanitizer build.  That file will still be compiled in normal builds
> > with all the warnings=errors enabled, so real problems won't be
> > overlooked.
>
> OK, we can try this path.
>
> Santosh, could you please revert and try -Wno-error + upstream it?
>
> Thank you in advance!
>

Sure, let me prepare the patch.

--
Santhosh


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Kamil Rytarowski
On 13.02.2020 22:20, Valery Ushakov wrote:
> I did not propose to disable the warning.  I proposed to downgrade
> -Werror to -Wno-error (i.e. a warning) and only for the buggy
> sanitizer build.  That file will still be compiled in normal builds
> with all the warnings=errors enabled, so real problems won't be
> overlooked.

OK, we can try this path.

Santosh, could you please revert and try -Wno-error + upstream it?

Thank you in advance!



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Valery Ushakov
On Thu, Feb 13, 2020 at 18:25:41 +0100, Kamil Rytarowski wrote:

> On 13.02.2020 18:00, Valery Ushakov wrote:
> > Arguably, if the tool you use is broken, you shouln't be mutilating
> > the source code just to shut that tool up. 
> 
> The introduced changes were good, even if GCC would be silent.

You changed one occurence just because it happens to trigger a bug in
gcc.  There are ntohs() >< size_t_var comparisons right above and
below the line you changed, where the same promotion happens (without
triggering a gcc bug) and they don't have a cast.  So someone reading
that code will ask themself, wait a minute, why the cast is necessary
in *that* place but not in those places?  What is going on there that
I miss that required introduction of that cast.  I.e. your change
create cognitive load for the reader.  I don't cosider that good.


> [...] the promotion rules are considered by many people as the major
> flaw in C.  Today I prefer explicit casts (after the MUSL lesson)
> even if unnecessary than implicit promotion.

Amen.  However they are there and we made uneasy peace with them so
unless you are consistent in your casting, you send all the wrong
singals to the reader.


> As an alternative option we can disable warnings but this is in my
> opinion much worse in this case than potentially overlooking real
> problems in a 4000+ line file.

I did not propose to disable the warning.  I proposed to downgrade
-Werror to -Wno-error (i.e. a warning) and only for the buggy
sanitizer build.  That file will still be compiled in normal builds
with all the warnings=errors enabled, so real problems won't be
overlooked.


> Repeatedly informing that the tool (GCC) is broken did not solve any
> issue.  It would be finally better to see someone fixing GCC rather
> than informing other GCC users about it.

Feel free to follow your own advice here...  (After all, it's your
sanitizer usage that has problems.  The normal builds are fine as they
are :)


> On the opposite side of this if this camp is MUSL. I used chunks of the
> MUSL code and it had poor results. There is a strict policy to avoid
> casts unless absolutely needed

That's not a bad policy.  As I said, a cast is a blunt tool.  It's
easy to introduce a wrong one that would silence some warning or other
but will do the wrong thing, but now since there is a cast the
compiler cannot even squeak.  I might be misremembering, but IIRC
Visual C has some warning(s) that ignore explicit casts, precisely to
detect that kind of problems.


> and if they are needed this tends to be in as obscure way as
> possible (like adding U attribute to one of the arguments in an
> expression).

What's obscure about adding 'U' to signify unsignedness?!  Also it's
not a cast to begin with, a literal with the 'u' suffix *is* unsigned.
A cast is when you take something of one type and coerce it to be of
some other type.


-uwe


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Kamil Rytarowski
On 13.02.2020 18:00, Valery Ushakov wrote:
> Arguably, if the tool you use is broken, you shouln't be mutilating
> the source code just to shut that tool up. 

The introduced changes were good, even if GCC would be silent. It is far
from mutilating. As an alternative option we can disable warnings but
this is in my opinion much worse in this case than potentially
overlooking real problems in a 4000+ line file.

On the opposite side of this if this camp is MUSL. I used chunks of the
MUSL code and it had poor results. There is a strict policy to avoid
casts unless absolutely needed and if they are needed this tends to be
in as obscure way as possible (like adding U attribute to one of the
arguments in an expression). This resulted in unmaintainable code and
very difficult situation to guess whether code semantics is broken or
buggy. For purists, MUSL is not mutilated at all so people wanting this
style are informed where to find it now.

Today I prefer explicit casts (after the MUSL lesson) even if
unnecessary than implicit promotion. I'm not alone in here as the
promotion rules are considered by many people as the major flaw in C.

Everybody agrees that GCC was picky in that case and not mandatory from
a strict language point of view.

Repeatedly informing that the tool (GCC) is broken did not solve any
issue. It would be finally better to see someone fixing GCC rather than
informing other GCC users about it.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Valery Ushakov
On Thu, Feb 13, 2020 at 15:03:35 +0100, Kamil Rytarowski wrote:

> On 13.02.2020 14:50, Valery Ushakov wrote:
> > On Thu, Feb 13, 2020 at 14:18:43 +0100, Kamil Rytarowski wrote:
> > 
> > Can you show the original problem that you are trying to fix here?
> > When and how did you get warning?
> 
> GCC has a property (as called by Joerg bug) that different backend,
> different optimization levels etc emit different warnings.
> 
> There is a request from certain people around to fix GCC not to mute
> down GCC, only because warnings are emitted in one configuration and not
> in their build settings.
> 
> I redirect these complains to GCC upstream.
> 
> dhcpcd emits these warnings once we enable -fsanitize=undefined.

If I add -fsanitize=undefined to the command line to compile that file
(in current) I still do not see that warning.  I didn't have time yet
to do a full build with sanitizer enabled.

Arguably, if the tool you use is broken, you shouln't be mutilating
the source code just to shut that tool up.  In general I'm quite
worried about the recent ongoing activity of sprinkling changes around
the tree just to shut up this or that warning without paying much
attention to the end result.  Some of those changes were wrong. Some
left code looking inconsistent as they fixed one place that did
produce a warning, but didn't change nearly identical bits of code
just next to it that didn't.  That leaves code in a fractured state.
Casts in particular are a very blunt instrument, and given that
history of recent changes your use of that instrument in this case
really raised red flags.

If -- and I still haven't seen it myself, "works for me" in a naive
test -- -fsanitize=undefined is buggy w.r.t. -Wconversion, then you
should arange to use say -Wno-error=conversion for that specific file
when -fsanitize=undefined is enabled, we do have makefile machinery
for that.  This way the bug in the tool is not causing random and
dubious changes to the code, and is confined to a properly commented
change in a Makefile.

-uwe


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Kamil Rytarowski
On 13.02.2020 14:50, Valery Ushakov wrote:
> On Thu, Feb 13, 2020 at 14:18:43 +0100, Kamil Rytarowski wrote:
> 
>> 
> Can you show the original problem that you are trying to fix here?
> When and how did you get warning?
> 

GCC has a property (as called by Joerg bug) that different backend,
different optimization levels etc emit different warnings.

There is a request from certain people around to fix GCC not to mute
down GCC, only because warnings are emitted in one configuration and not
in their build settings.

I redirect these complains to GCC upstream.

dhcpcd emits these warnings once we enable -fsanitize=undefined.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Valery Ushakov
On Thu, Feb 13, 2020 at 14:18:43 +0100, Kamil Rytarowski wrote:

> Feel free to fix GCC.

That's sounds rather dismissive to me.

Can you show the original problem that you are trying to fix here?
When and how did you get warning?

I do NOT see the warning for this kind of conversion with either
current (gcc 8) or on netbsd-8 (gcc 5).

If I revert this change the file is compiled just fine.  E.g. with a
very current current build:

$ nbmake-sparc dhcp.o
#   compile  dhcpcd/dhcp.o
sparc--netbsdelf-gcc -O2 ... -Wconversion -Wsign-compare -c .../dhcp.c
$



> dhcpcd already uses this explicit cast style, e.g. here:
>
> static void *
> get_udp_data(void *packet, size_t *len)
> {
> const struct ip *ip = packet;
> size_t ip_hl = (size_t)ip->ip_hl * 4;
> char *p = packet;
> 
> p += ip_hl + sizeof(struct udphdr);
> *len = (size_t)ntohs(ip->ip_len) - sizeof(struct udphdr) - ip_hl;
> return p;
> }


Again, I don't get any warnings if I change that to

   ... /*(size_t)*/ntohs(ip->ip_len) ...

I also tried 

   ... /*(size_t)ntohs*/(ip->ip_len) ...

as a quick and dirty attempt to emulate little endian where ntohs is a
nop (I don't have an up-to-date little endian build handy) and, again,
there's no warning.

Oh, actually, my tooldir has accreted quite a number of old superh
compilers, so:

$ cat size.c
typedef unsigned short uint16_t;
typedef unsigned long size_t;
#define ntohs(x) (x)

size_t
foo(uint16_t ip_len)
{
return ntohs(ip_len) - sizeof(uint16_t);
}
$ for CC in $TOOLDIR/bin/shle--netbsdelf-gcc-[0-9]*; do echo $(basename $CC); 
$CC -Wall -Wextra -Wconversion -Wsign-compare -O2 -S size.c; done
shle--netbsdelf-gcc-4.5.4
shle--netbsdelf-gcc-4.8.3
shle--netbsdelf-gcc-4.8.4
shle--netbsdelf-gcc-4.8.5
shle--netbsdelf-gcc-5.4.0
shle--netbsdelf-gcc-5.5.0
shle--netbsdelf-gcc-6.4.0
shle--netbsdelf-gcc-6.5.0
shle--netbsdelf-gcc-7.4.0
$

-uwe


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Kamil Rytarowski
On 13.02.2020 14:13, Joerg Sonnenberger wrote:
> On Thu, Feb 13, 2020 at 02:05:12PM +0100, Kamil Rytarowski wrote:
>> On 13.02.2020 00:58, Joerg Sonnenberger wrote:
>>> On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
 On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
>> Module Name: src
>> Committed By:fox
>> Date:Sat Feb  8 12:17:16 UTC 2020
>>
>> Modified Files:
>>  src/external/bsd/dhcpcd/dist/src: dhcp.c
>>
>> Log Message:
>> external/bsd/dhcpcd: Fix a -Wconversion warning.
>>
>> Type cast uint16_t to size_t to prevent implicit type conversion.
>
> Seriously? That should not warn and no cast should be used either.

 What fix would you recommend then?
>>>
>>> Disable the warning in GCC and fill an upstream PR against it. A
>>> conversion from uint16_t to size_t is value preserving by definition of
>>> the ISO C platform limits. It should never create a warning.
>>>
>>> Joerg
>>>
>>
>> If we disable warnings in our core software in non-trivial source file
>> for over 4000 lines we will mute more serious issues in a pretty
>> sensitive file. We fixed this warning in upstream dhcpcd.
> 
> The fix is IMO significantly worse as it actually creates technical
> debt. Frankly, I would go so far and say that this should be reverted.
> This warning seems to be pretty fundamentally broken by design if the
> analysis so far is correct.
> 
> Joerg
> 

Feel free to fix GCC.

dhcpcd already uses this explicit cast style, e.g. here:

static void *
get_udp_data(void *packet, size_t *len)
{
const struct ip *ip = packet;
size_t ip_hl = (size_t)ip->ip_hl * 4;
char *p = packet;

p += ip_hl + sizeof(struct udphdr);
*len = (size_t)ntohs(ip->ip_len) - sizeof(struct udphdr) - ip_hl;
return p;
}



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Joerg Sonnenberger
On Thu, Feb 13, 2020 at 02:05:12PM +0100, Kamil Rytarowski wrote:
> On 13.02.2020 00:58, Joerg Sonnenberger wrote:
> > On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> >> On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> >>> On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
>  Module Name: src
>  Committed By:fox
>  Date:Sat Feb  8 12:17:16 UTC 2020
> 
>  Modified Files:
>   src/external/bsd/dhcpcd/dist/src: dhcp.c
> 
>  Log Message:
>  external/bsd/dhcpcd: Fix a -Wconversion warning.
> 
>  Type cast uint16_t to size_t to prevent implicit type conversion.
> >>>
> >>> Seriously? That should not warn and no cast should be used either.
> >>
> >> What fix would you recommend then?
> > 
> > Disable the warning in GCC and fill an upstream PR against it. A
> > conversion from uint16_t to size_t is value preserving by definition of
> > the ISO C platform limits. It should never create a warning.
> > 
> > Joerg
> > 
> 
> If we disable warnings in our core software in non-trivial source file
> for over 4000 lines we will mute more serious issues in a pretty
> sensitive file. We fixed this warning in upstream dhcpcd.

The fix is IMO significantly worse as it actually creates technical
debt. Frankly, I would go so far and say that this should be reverted.
This warning seems to be pretty fundamentally broken by design if the
analysis so far is correct.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Kamil Rytarowski
On 13.02.2020 00:58, Joerg Sonnenberger wrote:
> On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
>> On 09/02/2020 19:21, Joerg Sonnenberger wrote:
>>> On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
 Module Name:   src
 Committed By:  fox
 Date:  Sat Feb  8 12:17:16 UTC 2020

 Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

 Log Message:
 external/bsd/dhcpcd: Fix a -Wconversion warning.

 Type cast uint16_t to size_t to prevent implicit type conversion.
>>>
>>> Seriously? That should not warn and no cast should be used either.
>>
>> What fix would you recommend then?
> 
> Disable the warning in GCC and fill an upstream PR against it. A
> conversion from uint16_t to size_t is value preserving by definition of
> the ISO C platform limits. It should never create a warning.
> 
> Joerg
> 

If we disable warnings in our core software in non-trivial source file
for over 4000 lines we will mute more serious issues in a pretty
sensitive file. We fixed this warning in upstream dhcpcd.

We ship with GCC that is typically 2 major versions behind upstream GCC
and this at least discourages handling generic issues directly with
upstream and upstream has little interest in reiterating over old
branches for corner cases that are generally speaking unimportant.

So overall I disagree with the request. Feel free to contribute a patch
to GCC or find someone in GCC to fix it (we have no resources to do it
on our side) so we can remove the cast.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Robert Elz
Date:Thu, 13 Feb 2020 02:45:44 +
From:Roy Marples 
Message-ID:  

  | My understanding was if it could be promoted to an int it would be.
  | So it size_t is bigger in bits than uint16_t and int is also bigger then 
  | promotion occurs and we then have signed vs unsigned.

You're right, but I think Joerg is even more right - or I certainly hope so.

The reasoning is that when the uint16_t gets promoted to int, since int
has more bits than the uint16_t the conversion is done by filling the
low 16 bits of the int with the uint16_t value, and zero filling the rest
(that is, value preserving).   You then have a signed int.  But it is
known to be >= 0.   When that is converted to a size_t (unsigned, and
here assumed to be at least as many bits as an int .. similar reasoning
applies if not) the signed int is sign extended to fit the size_t, and then
made unsigned.   "Sign exiended" here means zero fill, as we know the
value must be >= 0 (so the sign bit must be 0).

The only slightly weird case would be if int were 16 bits, but in that case,
a uint16_t would be an unsigned int, and no promotion to int ever happens,
it simply gets promoted directly to size_t.

If I had to guess, I'd say that gcc is losing the "must be >= 0" part of
the conversion from uint16_t to int, and assuming that the int might be
negative, which would be bad for converting to a size_t.

kre



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Roy Marples

On 13/02/2020 02:17, Joerg Sonnenberger wrote:

I thought this fell under int promotion and thus became signed vs unsigned?


size_t is guaranteed to be at least 16bit. If INT_MAX == 32767, an
implicit cast of uint16_t would go to unsigned anyway and in all other
cases, any implicit cast must be value preserving.


My understanding was if it could be promoted to an int it would be.
So it size_t is bigger in bits than uint16_t and int is also bigger then 
promotion occurs and we then have signed vs unsigned.


Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Joerg Sonnenberger
On Thu, Feb 13, 2020 at 02:07:23AM +, Roy Marples wrote:
> On 12/02/2020 23:58, Joerg Sonnenberger wrote:
> > On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> > > On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> > > > On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> > > > > Module Name:  src
> > > > > Committed By: fox
> > > > > Date: Sat Feb  8 12:17:16 UTC 2020
> > > > > 
> > > > > Modified Files:
> > > > >   src/external/bsd/dhcpcd/dist/src: dhcp.c
> > > > > 
> > > > > Log Message:
> > > > > external/bsd/dhcpcd: Fix a -Wconversion warning.
> > > > > 
> > > > > Type cast uint16_t to size_t to prevent implicit type conversion.
> > > > 
> > > > Seriously? That should not warn and no cast should be used either.
> > > 
> > > What fix would you recommend then?
> > 
> > Disable the warning in GCC and fill an upstream PR against it. A
> > conversion from uint16_t to size_t is value preserving by definition of
> > the ISO C platform limits. It should never create a warning.
> 
> I thought this fell under int promotion and thus became signed vs unsigned?

size_t is guaranteed to be at least 16bit. If INT_MAX == 32767, an
implicit cast of uint16_t would go to unsigned anyway and in all other
cases, any implicit cast must be value preserving.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Roy Marples

On 12/02/2020 23:58, Joerg Sonnenberger wrote:

On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:

On 09/02/2020 19:21, Joerg Sonnenberger wrote:

On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:

Module Name:src
Committed By:   fox
Date:   Sat Feb  8 12:17:16 UTC 2020

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
external/bsd/dhcpcd: Fix a -Wconversion warning.

Type cast uint16_t to size_t to prevent implicit type conversion.


Seriously? That should not warn and no cast should be used either.


What fix would you recommend then?


Disable the warning in GCC and fill an upstream PR against it. A
conversion from uint16_t to size_t is value preserving by definition of
the ISO C platform limits. It should never create a warning.


I thought this fell under int promotion and thus became signed vs unsigned?

Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Joerg Sonnenberger
On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> > On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> > > Module Name:  src
> > > Committed By: fox
> > > Date: Sat Feb  8 12:17:16 UTC 2020
> > > 
> > > Modified Files:
> > >   src/external/bsd/dhcpcd/dist/src: dhcp.c
> > > 
> > > Log Message:
> > > external/bsd/dhcpcd: Fix a -Wconversion warning.
> > > 
> > > Type cast uint16_t to size_t to prevent implicit type conversion.
> > 
> > Seriously? That should not warn and no cast should be used either.
> 
> What fix would you recommend then?

Disable the warning in GCC and fill an upstream PR against it. A
conversion from uint16_t to size_t is value preserving by definition of
the ISO C platform limits. It should never create a warning.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-10 Thread Roy Marples

On 09/02/2020 19:21, Joerg Sonnenberger wrote:

On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:

Module Name:src
Committed By:   fox
Date:   Sat Feb  8 12:17:16 UTC 2020

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
external/bsd/dhcpcd: Fix a -Wconversion warning.

Type cast uint16_t to size_t to prevent implicit type conversion.


Seriously? That should not warn and no cast should be used either.


What fix would you recommend then?

Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-09 Thread Joerg Sonnenberger
On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> Module Name:  src
> Committed By: fox
> Date: Sat Feb  8 12:17:16 UTC 2020
> 
> Modified Files:
>   src/external/bsd/dhcpcd/dist/src: dhcp.c
> 
> Log Message:
> external/bsd/dhcpcd: Fix a -Wconversion warning.
> 
> Type cast uint16_t to size_t to prevent implicit type conversion.

Seriously? That should not warn and no cast should be used either.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-01-27 Thread Christos Zoulas
Thanks. Yes, I have the core-dump, no we should not remove the line...

Best,

christos

> On Jan 27, 2020, at 8:03 AM, Roy Marples  wrote:
> 
> On 27/01/2020 09:03, Roy Marples wrote:
>> On 26/01/2020 22:57, Christos Zoulas wrote:
>>> Module Name:src
>>> Committed By:christos
>>> Date:Sun Jan 26 22:57:52 UTC 2020
>>> 
>>> Modified Files:
>>> src/external/bsd/dhcpcd/dist/src: script.c
>>> 
>>> Log Message:
>>> prevent coredump when state == NULL
>>> 
>>> 
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.1.1.12 -r1.2 src/external/bsd/dhcpcd/dist/src/script.c
>> A quick perusal through the code shows that this should not happen. Every 
>> time IPV4LL is given as a reason, the state should exist.
> 
> There is one code path where there may be no state  when an IPv4LL 
> address exists and dhcpcd no longer controlls it (ie dhcpcd restarted or a 
> 3rd party added it) then we delete it anyway (for say when we obtain a DHCP 
> address).
> We should still run the IPV4LL hook script, but it will be empty.
> 
> I've accepted this patch upstream.
> 
> Roy



signature.asc
Description: Message signed with OpenPGP


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-01-27 Thread Roy Marples

On 27/01/2020 09:03, Roy Marples wrote:

On 26/01/2020 22:57, Christos Zoulas wrote:

Module Name:    src
Committed By:    christos
Date:    Sun Jan 26 22:57:52 UTC 2020

Modified Files:
src/external/bsd/dhcpcd/dist/src: script.c

Log Message:
prevent coredump when state == NULL


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.12 -r1.2 src/external/bsd/dhcpcd/dist/src/script.c


A quick perusal through the code shows that this should not happen. Every time 
IPV4LL is given as a reason, the state should exist.


There is one code path where there may be no state  when an IPv4LL address 
exists and dhcpcd no longer controlls it (ie dhcpcd restarted or a 3rd party 
added it) then we delete it anyway (for say when we obtain a DHCP address).

We should still run the IPV4LL hook script, but it will be empty.

I've accepted this patch upstream.

Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-01-27 Thread Roy Marples

On 26/01/2020 22:57, Christos Zoulas wrote:

Module Name:src
Committed By:   christos
Date:   Sun Jan 26 22:57:52 UTC 2020

Modified Files:
src/external/bsd/dhcpcd/dist/src: script.c

Log Message:
prevent coredump when state == NULL


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.12 -r1.2 src/external/bsd/dhcpcd/dist/src/script.c


A quick perusal through the code shows that this should not happen. Every time 
IPV4LL is given as a reason, the state should exist.
If there really is no state then the script should not be called at all - we 
wouldn't want to pass the error down to dhcpcd listeners who would expect either 
an old address or new address to be present.


Hopefully you still have the coredump so this can be analysed more?

Should we also remove this line fromm 3RDPARTY?
http://anonhg.netbsd.org/src/file/tip/doc/3RDPARTY#l340

Roy


CVS commit: src/external/bsd/dhcpcd/dist/src

2019-11-13 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Nov 13 10:50:22 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcp6.c dhcpcd.c if-bsd.c
ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.29 -r1.30 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.28 -r1.29 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.14 -r1.15 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.12 -r1.13 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-11-13 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Nov 13 10:50:22 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcp6.c dhcpcd.c if-bsd.c
ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.29 -r1.30 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.28 -r1.29 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.14 -r1.15 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.12 -r1.13 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.29 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.30
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.29	Wed Oct 16 14:54:39 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Wed Nov 13 10:50:22 2019
@@ -41,6 +41,10 @@
 #include 
 #undef __FAVOR_BSD
 
+#ifdef AF_LINK
+#  include 
+#endif
+
 #include 
 #include 
 #include 
@@ -132,9 +136,7 @@ static void dhcp_arp_found(struct arp_st
 #endif
 static void dhcp_handledhcp(struct interface *, struct bootp *, size_t,
 const struct in_addr *);
-#ifdef IP_PKTINFO
 static void dhcp_handleifudp(void *);
-#endif
 static int dhcp_initstate(struct interface *);
 
 void
@@ -1550,7 +1552,10 @@ dhcp_openudp(struct interface *ifp)
 	n = 1;
 	if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, , sizeof(n)) == -1)
 		goto eexit;
-#ifdef IP_RECVPKTINFO
+#ifdef IP_RECVIF
+	if (setsockopt(s, IPPROTO_IP, IP_RECVIF, , sizeof(n)) == -1)
+		goto eexit;
+#else
 	if (setsockopt(s, IPPROTO_IP, IP_RECVPKTINFO, , sizeof(n)) == -1)
 		goto eexit;
 #endif
@@ -1647,39 +1652,36 @@ dhcp_makeudppacket(size_t *sz, const uin
 static ssize_t
 dhcp_sendudp(struct interface *ifp, struct in_addr *to, void *data, size_t len)
 {
-	int s;
-	struct msghdr msg;
-	struct sockaddr_in sin;
-	struct iovec iov[1];
-	struct dhcp_state *state = D_STATE(ifp);
-	ssize_t r;
-
-	iov[0].iov_base = data;
-	iov[0].iov_len = len;
-
-	memset(, 0, sizeof(sin));
-	sin.sin_family = AF_INET;
-	sin.sin_addr = *to;
-	sin.sin_port = htons(BOOTPS);
+	struct sockaddr_in sin = {
+		.sin_family = AF_INET,
+		.sin_addr = *to,
+		.sin_port = htons(BOOTPS),
 #ifdef HAVE_SA_LEN
-	sin.sin_len = sizeof(sin);
+		.sin_len = sizeof(sin),
 #endif
+	};
+	struct iovec iov[] = {
+		{ .iov_base = data, .iov_len = len }
+	};
+	struct msghdr msg = {
+		.msg_name = (void *),
+		.msg_namelen = sizeof(sin),
+		.msg_iov = iov,
+		.msg_iovlen = 1,
+	};
+	struct dhcp_state *state = D_STATE(ifp);
+	ssize_t r;
+	int fd;
 
-	memset(, 0, sizeof(msg));
-	msg.msg_name = (void *)
-	msg.msg_namelen = sizeof(sin);
-	msg.msg_iov = iov;
-	msg.msg_iovlen = 1;
-
-	s = state->udp_fd;
-	if (s == -1) {
-		s = dhcp_openudp(ifp);
-		if (s == -1)
+	fd = state->udp_fd;
+	if (fd == -1) {
+		fd = dhcp_openudp(ifp);
+		if (fd == -1)
 			return -1;
 	}
-	r = sendmsg(s, , 0);
+	r = sendmsg(fd, , 0);
 	if (state->udp_fd == -1)
-		close(s);
+		close(fd);
 	return r;
 }
 
@@ -1780,7 +1782,7 @@ send_message(struct interface *ifp, uint
 	 * As such we remove it from consideration without actually
 	 * stopping the interface. */
 	if (r == -1) {
-		logerr("%s: if_sendraw", ifp->name);
+		logerr("%s: bpf_send", ifp->name);
 		switch(errno) {
 		case ENETDOWN:
 		case ENETRESET:
@@ -2257,30 +2259,27 @@ dhcp_bind(struct interface *ifp)
 
 	ipv4_applyaddr(ifp);
 
-#ifdef IP_PKTINFO
 	/* Close the BPF filter as we can now receive DHCP messages
 	 * on a UDP socket. */
-	if (state->udp_fd == -1 ||
-	(state->old != NULL && state->old->yiaddr != state->new->yiaddr))
-	{
-		dhcp_close(ifp);
-		/* If not in master mode, open an address specific socket. */
-		if (ctx->udp_fd == -1) {
-			state->udp_fd = dhcp_openudp(ifp);
-			if (state->udp_fd == -1) {
-logerr(__func__);
-/* Address sharing without master mode is
- * not supported. It's also possible another
- * DHCP client could be running which is
- * even worse.
- * We still need to work, so re-open BPF. */
-dhcp_openbpf(ifp);
-			} else
-eloop_event_add(ctx->eloop,
-state->udp_fd, dhcp_handleifudp, ifp);
-		}
+	if (!(state->udp_fd == -1 ||
+	(state->old != NULL && state->old->yiaddr != state->new->yiaddr)))
+		return;
+	dhcp_close(ifp);
+
+	/* If not in master mode, open an address specific socket. */
+	if (ctx->udp_fd != -1)
+		return;
+	state->udp_fd = dhcp_openudp(ifp);
+	if (state->udp_fd == -1) {
+		logerr(__func__);
+		/* Address sharing without master mode is not supported.
+		 * It's also possible another DHCP client could be running,
+		 * which is even worse.
+		 * We still need to work, so re-open BPF. */
+		dhcp_openbpf(ifp);
+		return;
 	}
-#endif
+	eloop_event_add(ctx->eloop, state->udp_fd, dhcp_handleifudp, ifp);
 }
 
 static void
@@ -2609,6 +2608,11 @@ dhcp_reboot(struct interface 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-10-16 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Oct 16 14:54:39 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcpcd.c ipv6.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.28 -r1.29 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.27 -r1.28 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.5 -r1.6 src/external/bsd/dhcpcd/dist/src/ipv6.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.28 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.29
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.28	Fri Oct 11 11:03:59 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Wed Oct 16 14:54:39 2019
@@ -164,8 +164,6 @@ dhcp_printoptions(const struct dhcpcd_ct
 	}
 }
 
-#define get_option_raw(ctx, bootp, bootp_len, opt)	\
-	get_option((ctx), (bootp), (bootp_len), NULL)
 static const uint8_t *
 get_option(struct dhcpcd_ctx *ctx,
 const struct bootp *bootp, size_t bootp_len,
@@ -3275,26 +3273,35 @@ is_packet_udp_bootp(void *packet, size_t
 {
 	struct ip *ip = packet;
 	size_t ip_hlen;
-	struct udphdr *udp;
+	struct udphdr udp;
 
-	if (sizeof(*ip) > plen)
+	if (plen < sizeof(*ip))
 		return false;
 
 	if (ip->ip_v != IPVERSION || ip->ip_p != IPPROTO_UDP)
 		return false;
 
 	/* Sanity. */
-	if (ntohs(ip->ip_len) != plen)
+	if (ntohs(ip->ip_len) > plen)
 		return false;
 
 	ip_hlen = (size_t)ip->ip_hl * 4;
+	if (ip_hlen < sizeof(*ip))
+		return false;
+
 	/* Check we have a UDP header and BOOTP. */
-	if (ip_hlen + sizeof(*udp) + offsetof(struct bootp, vend) > plen)
+	if (ip_hlen + sizeof(udp) + offsetof(struct bootp, vend) > plen)
+		return false;
+
+	/* Sanity. */
+	memcpy(, (char *)ip + ip_hlen, sizeof(udp));
+	if (ntohs(udp.uh_ulen) < sizeof(udp))
+		return false;
+	if (ip_hlen + ntohs(udp.uh_ulen) > plen)
 		return false;
 
 	/* Check it's to and from the right ports. */
-	udp = (struct udphdr *)(void *)((char *)ip + ip_hlen);
-	if (udp->uh_dport != htons(BOOTPC) || udp->uh_sport != htons(BOOTPS))
+	if (udp.uh_dport != htons(BOOTPC) || udp.uh_sport != htons(BOOTPS))
 		return false;
 
 	return true;
@@ -3306,14 +3313,17 @@ checksums_valid(void *packet,
 struct in_addr *from, unsigned int flags)
 {
 	struct ip *ip = packet;
-	struct ip pseudo_ip = {
-		.ip_p = IPPROTO_UDP,
-		.ip_src = ip->ip_src,
-		.ip_dst = ip->ip_dst
+	union pip {
+		struct ip ip;
+		uint16_t w[sizeof(struct ip)];
+	} pip = {
+		.ip.ip_p = IPPROTO_UDP,
+		.ip.ip_src = ip->ip_src,
+		.ip.ip_dst = ip->ip_dst,
 	};
 	size_t ip_hlen;
-	uint16_t udp_len, uh_sum;
-	struct udphdr *udp;
+	struct udphdr udp;
+	char *udpp, *uh_sump;
 	uint32_t csum;
 
 	if (from != NULL)
@@ -3324,22 +3334,32 @@ checksums_valid(void *packet,
 		return false;
 
 	if (flags & BPF_PARTIALCSUM)
-		return 0;
+		return true;
 
-	udp = (struct udphdr *)(void *)((char *)ip + ip_hlen);
-	if (udp->uh_sum == 0)
-		return 0;
+	udpp = (char *)ip + ip_hlen;
+	memcpy(, udpp, sizeof(udp));
+	if (udp.uh_sum == 0)
+		return true;
 
 	/* UDP checksum is based on a pseudo IP header alongside
 	 * the UDP header and payload. */
-	udp_len = ntohs(udp->uh_ulen);
-	uh_sum = udp->uh_sum;
-	udp->uh_sum = 0;
-	pseudo_ip.ip_len = udp->uh_ulen;
+	pip.ip.ip_len = udp.uh_ulen;
 	csum = 0;
-	in_cksum(_ip, sizeof(pseudo_ip), );
-	csum = in_cksum(udp, udp_len, );
-	return csum == uh_sum;
+
+	/* Need to zero the UDP sum in the packet for the checksum to work. */
+	uh_sump = udpp + offsetof(struct udphdr, uh_sum);
+	memset(uh_sump, 0, sizeof(udp.uh_sum));
+
+	/* Checksum psuedo header and then UDP + payload. */
+	in_cksum(pip.w, sizeof(pip.w), );
+	csum = in_cksum(udpp, ntohs(udp.uh_ulen), );
+
+#if 0	/* Not needed, just here for completeness. */
+	/* Put the checksum back. */
+	memcpy(uh_sump, _sum, sizeof(udp.uh_sum));
+#endif
+
+	return csum == udp.uh_sum;
 }
 
 static void

Index: src/external/bsd/dhcpcd/dist/src/dhcpcd.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.27 src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.28
--- src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.27	Fri Oct 11 11:03:59 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcpcd.c	Wed Oct 16 14:54:39 2019
@@ -1984,12 +1984,10 @@ printpidfile:
 	logdebugx(PACKAGE "-" VERSION " starting");
 	ctx.options |= DHCPCD_STARTED;
 
-#ifdef HAVE_SETPROCTITLE
 	setproctitle("%s%s%s",
 	ctx.options & DHCPCD_MASTER ? "[master]" : argv[optind],
 	ctx.options & DHCPCD_IPV4 ? " [ip4]" : "",
 	ctx.options & DHCPCD_IPV6 ? " [ip6]" : "");
-#endif
 
 	if (if_opensockets() == -1) {
 		logerr("%s: if_opensockets", __func__);
@@ -2155,6 +2153,9 @@ exit1:
 		loginfox(PACKAGE " exited");
 	logclose();
 	free(ctx.logfile);
+#ifdef SETPROCTITLE_H
+	setproctitle_free();
+#endif
 #ifdef USE_SIGNALS
 	if (ctx.options & DHCPCD_FORKED)
 		_exit(i); /* so atexit won't remove our pidfile */

Index: 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-10-16 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Oct 16 14:54:39 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcpcd.c ipv6.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.28 -r1.29 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.27 -r1.28 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.5 -r1.6 src/external/bsd/dhcpcd/dist/src/ipv6.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-10-11 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Oct 11 11:03:59 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcp6.c dhcpcd.8.in
dhcpcd.c if-bsd.c if-options.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.13 src/external/bsd/dhcpcd/dist/src/bpf.c \
src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.27 -r1.28 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in
cvs rdiff -u -r1.26 -r1.27 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.17 -r1.18 src/external/bsd/dhcpcd/dist/src/if-options.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/bpf.c
diff -u src/external/bsd/dhcpcd/dist/src/bpf.c:1.12 src/external/bsd/dhcpcd/dist/src/bpf.c:1.13
--- src/external/bsd/dhcpcd/dist/src/bpf.c:1.12	Wed Aug 21 17:12:19 2019
+++ src/external/bsd/dhcpcd/dist/src/bpf.c	Fri Oct 11 11:03:59 2019
@@ -410,13 +410,7 @@ bpf_cmp_hwaddr(struct bpf_insn *bpf, siz
 #endif
 
 #ifdef ARP
-
 static const struct bpf_insn bpf_arp_ether [] = {
-	/* Ensure packet is at least correct size. */
-	BPF_STMT(BPF_LD + BPF_W + BPF_LEN, 0),
-	BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(struct ether_arp), 1, 0),
-	BPF_STMT(BPF_RET + BPF_K, 0),
-
 	/* Check this is an ARP packet. */
 	BPF_STMT(BPF_LD + BPF_H + BPF_ABS,
 	 offsetof(struct ether_header, ether_type)),
@@ -552,17 +546,8 @@ bpf_arp(struct interface *ifp, int fd)
 }
 #endif
 
-#define	BPF_M_FHLEN	0
-#define	BPF_M_IPHLEN	1
-#define	BPF_M_IPLEN	2
-#define	BPF_M_UDP	3
-#define	BPF_M_UDPLEN	4
-
 #ifdef ARPHRD_NONE
 static const struct bpf_insn bpf_bootp_none[] = {
-	/* Set the frame header length to zero. */
-	BPF_STMT(BPF_LD + BPF_IMM, 0),
-	BPF_STMT(BPF_ST, BPF_M_FHLEN),
 };
 #define BPF_BOOTP_NONE_LEN	__arraycount(bpf_bootp_none)
 #endif
@@ -574,13 +559,14 @@ static const struct bpf_insn bpf_bootp_e
 	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETHERTYPE_IP, 1, 0),
 	BPF_STMT(BPF_RET + BPF_K, 0),
 
-	/* Load frame header length into X. */
-	BPF_STMT(BPF_LDX + BPF_W + BPF_IMM, sizeof(struct ether_header)),
-	/* Copy frame header length to memory */
-	BPF_STMT(BPF_STX, BPF_M_FHLEN),
+	/* Advance to the IP header. */
+	BPF_STMT(BPF_LDX + BPF_K, sizeof(struct ether_header)),
 };
 #define BPF_BOOTP_ETHER_LEN	__arraycount(bpf_bootp_ether)
 
+#define BOOTP_MIN_SIZE		sizeof(struct ip) + sizeof(struct udphdr) + \
+sizeof(struct bootp)
+
 static const struct bpf_insn bpf_bootp_filter[] = {
 	/* Make sure it's an IPv4 packet. */
 	BPF_STMT(BPF_LD + BPF_B + BPF_IND, 0),
@@ -588,15 +574,6 @@ static const struct bpf_insn bpf_bootp_f
 	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x40, 1, 0),
 	BPF_STMT(BPF_RET + BPF_K, 0),
 
-	/* Ensure IP header length is big enough and
-	 * store the IP header length in memory. */
-	BPF_STMT(BPF_LD + BPF_B + BPF_IND, 0),
-	BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x0f),
-	BPF_STMT(BPF_ALU + BPF_MUL + BPF_K, 4),
-	BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(struct ip), 1, 0),
-	BPF_STMT(BPF_RET + BPF_K, 0),
-	BPF_STMT(BPF_ST, BPF_M_IPHLEN),
-
 	/* Make sure it's a UDP packet. */
 	BPF_STMT(BPF_LD + BPF_B + BPF_IND, offsetof(struct ip, ip_p)),
 	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 1, 0),
@@ -607,49 +584,17 @@ static const struct bpf_insn bpf_bootp_f
 	BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 0, 1),
 	BPF_STMT(BPF_RET + BPF_K, 0),
 
-	/* Store IP length. */
-	BPF_STMT(BPF_LD + BPF_H + BPF_IND, offsetof(struct ip, ip_len)),
-	BPF_STMT(BPF_ST, BPF_M_IPLEN),
-
 	/* Advance to the UDP header. */
-	BPF_STMT(BPF_LD + BPF_MEM, BPF_M_IPHLEN),
+	BPF_STMT(BPF_LD + BPF_B + BPF_IND, 0),
+	BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x0f),
+	BPF_STMT(BPF_ALU + BPF_MUL + BPF_K, 4),
 	BPF_STMT(BPF_ALU + BPF_ADD + BPF_X, 0),
 	BPF_STMT(BPF_MISC + BPF_TAX, 0),
 
-	/* Store UDP location */
-	BPF_STMT(BPF_STX, BPF_M_UDP),
-
 	/* Make sure it's from and to the right port. */
 	BPF_STMT(BPF_LD + BPF_W + BPF_IND, 0),
 	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, (BOOTPS << 16) + BOOTPC, 1, 0),
 	BPF_STMT(BPF_RET + BPF_K, 0),
-
-	/* Store UDP length. */
-	BPF_STMT(BPF_LD + BPF_H + BPF_IND, offsetof(struct udphdr, uh_ulen)),
-	BPF_STMT(BPF_ST, BPF_M_UDPLEN),
-
-	/* Ensure that UDP length + IP header length == IP length */
-	/* Copy IP header length to X. */
-	BPF_STMT(BPF_LDX + BPF_MEM, BPF_M_IPHLEN),
-	/* Add UDP length (A) to IP header length (X). */
-	BPF_STMT(BPF_ALU + BPF_ADD + BPF_X, 0),
-	/* Store result in X. */
-	BPF_STMT(BPF_MISC + BPF_TAX, 0),
-	/* Copy IP length to A. */
-	BPF_STMT(BPF_LD + BPF_MEM, BPF_M_IPLEN),
-	/* Ensure X == A. */
-	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_X, 0, 1, 0),
-	BPF_STMT(BPF_RET + BPF_K, 0),
-
-	/* Advance to the BOOTP packet. */
-	BPF_STMT(BPF_LD + BPF_MEM, BPF_M_UDP),
-	BPF_STMT(BPF_ALU + BPF_ADD + BPF_K, 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-10-11 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Oct 11 11:03:59 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcp6.c dhcpcd.8.in
dhcpcd.c if-bsd.c if-options.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.12 -r1.13 src/external/bsd/dhcpcd/dist/src/bpf.c \
src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.27 -r1.28 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in
cvs rdiff -u -r1.26 -r1.27 src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.17 -r1.18 src/external/bsd/dhcpcd/dist/src/if-options.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-09-13 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Sep 13 11:54:03 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.26 -r1.27 src/external/bsd/dhcpcd/dist/src/dhcp.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.26 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.27
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.26	Fri Sep 13 11:01:50 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Fri Sep 13 11:54:03 2019
@@ -3264,7 +3264,13 @@ valid_udp_packet(void *packet, size_t pl
 	if (from != NULL)
 		from->s_addr = ip->ip_src.s_addr;
 
+	/* Check we have the IP header */
 	ip_hlen = (size_t)ip->ip_hl * 4;
+	if (ip_hlen > plen) {
+		errno = ENOBUFS;
+		return -1;
+	}
+
 	if (in_cksum(ip, ip_hlen, NULL) != 0) {
 		errno = EINVAL;
 		return -1;



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-09-13 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Sep 13 11:54:03 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.26 -r1.27 src/external/bsd/dhcpcd/dist/src/dhcp.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-09-04 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Sep  4 13:28:57 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcp6.c dhcpcd.8.in dhcpcd.c
if-bsd.c ipv6.c ipv6.h ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.24 -r1.25 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/dhcpcd/dist/src/dhcp6.c \
src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.2 -r1.3 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/ipv6.c
cvs rdiff -u -r1.4 -r1.5 src/external/bsd/dhcpcd/dist/src/ipv6.h
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-09-04 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Sep  4 13:28:57 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcp6.c dhcpcd.8.in dhcpcd.c
if-bsd.c ipv6.c ipv6.h ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.24 -r1.25 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/dhcpcd/dist/src/dhcp6.c \
src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.2 -r1.3 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/ipv6.c
cvs rdiff -u -r1.4 -r1.5 src/external/bsd/dhcpcd/dist/src/ipv6.h
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.24 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.25
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.24	Wed Aug 21 17:12:19 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Wed Sep  4 13:28:56 2019
@@ -1176,11 +1176,8 @@ read_lease(struct interface *ifp, struct
 	bytes = dhcp_read_lease_fd(fd, (void **));
 	if (fd_opened)
 		close(fd);
-	if (bytes == 0) {
-		free(lease);
-		logerr("%s: dhcp_read_lease_fd", __func__);
+	if (bytes == 0)
 		return 0;
-	}
 
 	/* Ensure the packet is at lease BOOTP sized
 	 * with a vendor area of 4 octets
@@ -1584,7 +1581,7 @@ eexit:
 }
 
 static uint16_t
-in_cksum(void *data, size_t len, uint32_t *isum)
+in_cksum(const void *data, size_t len, uint32_t *isum)
 {
 	const uint16_t *word = data;
 	uint32_t sum = isum != NULL ? *isum : 0;
@@ -1593,7 +1590,7 @@ in_cksum(void *data, size_t len, uint32_
 		sum += *word++;
 
 	if (len == 1)
-		sum += *(const uint8_t *)word;
+		sum += htons((uint16_t)(*(const uint8_t *)word << 8));
 
 	if (isum != NULL)
 		*isum = sum;
@@ -2237,7 +2234,7 @@ dhcp_bind(struct interface *ifp)
 	ipv4_applyaddr(ifp);
 
 #ifdef IP_PKTINFO
-	/* Close the BPF filter as we can now receive the DHCP renew messages
+	/* Close the BPF filter as we can now receive DHCP messages
 	 * on a UDP socket. */
 	if (state->udp_fd == -1 ||
 	(state->old != NULL && state->old->yiaddr != state->new->yiaddr))
@@ -2246,9 +2243,15 @@ dhcp_bind(struct interface *ifp)
 		/* If not in master mode, open an address specific socket. */
 		if (ctx->udp_fd == -1) {
 			state->udp_fd = dhcp_openudp(ifp);
-			if (state->udp_fd == -1)
+			if (state->udp_fd == -1) {
 logerr(__func__);
-			else
+/* Address sharing without master mode is
+ * not supported. It's also possible another
+ * DHCP client could be running which is
+ * even worse.
+ * We still need to work, so re-open BPF. */
+dhcp_openbpf(ifp);
+			} else
 eloop_event_add(ctx->eloop,
 state->udp_fd, dhcp_handleifudp, ifp);
 		}
@@ -3293,7 +3296,8 @@ valid_udp_packet(void *packet, size_t pl
 	pseudo_ip.ip_len = udp->uh_ulen;
 	csum = 0;
 	in_cksum(_ip, sizeof(pseudo_ip), );
-	if (in_cksum(udp, ntohs(udp->uh_ulen), ) != uh_sum) {
+	csum = in_cksum(udp, ntohs(udp->uh_ulen), );
+	if (csum != uh_sum) {
 		errno = EINVAL;
 		return -1;
 	}
@@ -3662,6 +3666,7 @@ static void
 dhcp_start1(void *arg)
 {
 	struct interface *ifp = arg;
+	struct dhcpcd_ctx *ctx = ifp->ctx;
 	struct if_options *ifo = ifp->options;
 	struct dhcp_state *state;
 	struct stat st;
@@ -3672,17 +3677,19 @@ dhcp_start1(void *arg)
 		return;
 
 	/* Listen on *.*.*.*:bootpc so that the kernel never sends an
-	 * ICMP port unreachable message back to the DHCP server */
-	if (ifp->ctx->udp_fd == -1) {
-		ifp->ctx->udp_fd = dhcp_openudp(NULL);
-		if (ifp->ctx->udp_fd == -1) {
+	 * ICMP port unreachable message back to the DHCP server.
+	 * Only do this in master mode so we don't swallow messages
+	 * for dhcpcd running on another interface. */
+	if (ctx->udp_fd == -1 && ctx->options & DHCPCD_MASTER) {
+		ctx->udp_fd = dhcp_openudp(NULL);
+		if (ctx->udp_fd == -1) {
 			/* Don't log an error if some other process
 			 * is handling this. */
 			if (errno != EADDRINUSE)
 logerr("%s: dhcp_openudp", __func__);
 		} else
-			eloop_event_add(ifp->ctx->eloop,
-			ifp->ctx->udp_fd, dhcp_handleudp, ifp->ctx);
+			eloop_event_add(ctx->eloop,
+			ctx->udp_fd, dhcp_handleudp, ctx);
 	}
 
 	if (dhcp_init(ifp) == -1) {
Index: src/external/bsd/dhcpcd/dist/src/dhcpcd.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.24 src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.25
--- src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.24	Wed Aug 21 17:12:19 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcpcd.c	Wed Sep  4 13:28:56 2019
@@ -950,12 +950,7 @@ dhcpcd_prestartinterface(void *arg)
 
 	if ((!(ifp->ctx->options & DHCPCD_MASTER) ||
 	ifp->options->options & DHCPCD_IF_UP) &&
-	if_up(ifp) == -1
-#ifdef __sun
-	/* Interface could not yet be plumbed. */
-	&& 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-08-21 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Aug 21 17:12:19 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcpcd.c if-bsd.c
if-options.c ipv6.h

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/dhcpcd/dist/src/bpf.c
cvs rdiff -u -r1.23 -r1.24 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.16 -r1.17 src/external/bsd/dhcpcd/dist/src/if-options.c
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/ipv6.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/bpf.c
diff -u src/external/bsd/dhcpcd/dist/src/bpf.c:1.11 src/external/bsd/dhcpcd/dist/src/bpf.c:1.12
--- src/external/bsd/dhcpcd/dist/src/bpf.c:1.11	Tue Jul 30 10:25:03 2019
+++ src/external/bsd/dhcpcd/dist/src/bpf.c	Wed Aug 21 17:12:19 2019
@@ -558,6 +558,15 @@ bpf_arp(struct interface *ifp, int fd)
 #define	BPF_M_UDP	3
 #define	BPF_M_UDPLEN	4
 
+#ifdef ARPHRD_NONE
+static const struct bpf_insn bpf_bootp_none[] = {
+	/* Set the frame header length to zero. */
+	BPF_STMT(BPF_LD + BPF_IMM, 0),
+	BPF_STMT(BPF_ST, BPF_M_FHLEN),
+};
+#define BPF_BOOTP_NONE_LEN	__arraycount(bpf_bootp_none)
+#endif
+
 static const struct bpf_insn bpf_bootp_ether[] = {
 	/* Make sure this is an IP packet. */
 	BPF_STMT(BPF_LD + BPF_H + BPF_ABS,
@@ -665,6 +674,12 @@ bpf_bootp(struct interface *ifp, int fd)
 	bp = bpf;
 	/* Check frame header. */
 	switch(ifp->family) {
+#ifdef ARPHRD_NONE
+	case ARPHRD_NONE:
+		memcpy(bp, bpf_bootp_none, sizeof(bpf_bootp_none));
+		bp += BPF_BOOTP_NONE_LEN;
+		break;
+#endif
 	case ARPHRD_ETHER:
 		memcpy(bp, bpf_bootp_ether, sizeof(bpf_bootp_ether));
 		bp += BPF_BOOTP_ETHER_LEN;

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.23 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.24
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.23	Tue Jul 30 10:25:03 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Wed Aug 21 17:12:19 2019
@@ -1584,24 +1584,24 @@ eexit:
 }
 
 static uint16_t
-checksum(const void *data, size_t len)
+in_cksum(void *data, size_t len, uint32_t *isum)
 {
-	const uint8_t *addr = data;
-	uint32_t sum = 0;
+	const uint16_t *word = data;
+	uint32_t sum = isum != NULL ? *isum : 0;
 
-	while (len > 1) {
-		sum += (uint32_t)(addr[0] * 256 + addr[1]);
-		addr += 2;
-		len -= 2;
-	}
+	for (; len > 1; len -= sizeof(*word))
+		sum += *word++;
 
 	if (len == 1)
-		sum += (uint32_t)(*addr * 256);
+		sum += *(const uint8_t *)word;
+
+	if (isum != NULL)
+		*isum = sum;
 
 	sum = (sum >> 16) + (sum & 0x);
 	sum += (sum >> 16);
 
-	return (uint16_t)~htons((uint16_t)sum);
+	return (uint16_t)~sum;
 }
 
 static struct bootp_pkt *
@@ -1639,14 +1639,16 @@ dhcp_makeudppacket(size_t *sz, const uin
 	udp->uh_dport = htons(BOOTPS);
 	udp->uh_ulen = htons((uint16_t)(sizeof(*udp) + length));
 	ip->ip_len = udp->uh_ulen;
-	udp->uh_sum = checksum(udpp, sizeof(*ip) +  sizeof(*udp) + length);
+	udp->uh_sum = in_cksum(udpp, sizeof(*ip) + sizeof(*udp) + length, NULL);
 
 	ip->ip_v = IPVERSION;
 	ip->ip_hl = sizeof(*ip) >> 2;
 	ip->ip_id = (uint16_t)arc4random_uniform(UINT16_MAX);
 	ip->ip_ttl = IPDEFTTL;
 	ip->ip_len = htons((uint16_t)(sizeof(*ip) + sizeof(*udp) + length));
-	ip->ip_sum = checksum(ip, sizeof(*ip));
+	ip->ip_sum = in_cksum(ip, sizeof(*ip), NULL);
+	if (ip->ip_sum == 0)
+		ip->ip_sum = 0x; /* RFC 768 */
 
 	*sz = sizeof(*ip) + sizeof(*udp) + length;
 	return udpp;
@@ -2363,7 +2365,10 @@ dhcp_arp_address(struct interface *ifp)
 		return 0;
 	}
 #else
-	if (ifp->options->options & DHCPCD_ARP && ia == NULL) {
+	if (!(ifp->flags & IFF_NOARP) &&
+	ifp->options->options & DHCPCD_ARP &&
+	ia == NULL)
+	{
 		struct arp_state *astate;
 		struct dhcp_lease l;
 
@@ -3236,10 +3241,15 @@ valid_udp_packet(void *packet, size_t pl
 	unsigned int flags)
 {
 	struct ip *ip = packet;
-	char ip_hlv = *(char *)ip;
+	struct ip pseudo_ip = {
+		.ip_p = IPPROTO_UDP,
+		.ip_src = ip->ip_src,
+		.ip_dst = ip->ip_dst
+	};
 	size_t ip_hlen;
 	uint16_t ip_len, uh_sum;
 	struct udphdr *udp;
+	uint32_t csum;
 
 	if (plen < sizeof(*ip)) {
 		if (from != NULL)
@@ -3252,13 +3262,13 @@ valid_udp_packet(void *packet, size_t pl
 		from->s_addr = ip->ip_src.s_addr;
 
 	ip_hlen = (size_t)ip->ip_hl * 4;
-	if (checksum(ip, ip_hlen) != 0) {
+	if (in_cksum(ip, ip_hlen, NULL) != 0) {
 		errno = EINVAL;
 		return -1;
 	}
 
-	ip_len = ntohs(ip->ip_len);
 	/* Check we have a payload */
+	ip_len = ntohs(ip->ip_len);
 	if (ip_len <= ip_hlen + sizeof(*udp)) {
 		errno = ERANGE;
 		return -1;
@@ -3272,28 +3282,21 @@ valid_udp_packet(void *packet, size_t pl
 	if (flags & BPF_PARTIALCSUM)
 		return 0;
 
-	udp = (struct udphdr *)((char *)ip + ip_hlen);
+	/* UDP checksum is based on a pseudo IP 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-08-21 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Aug 21 17:12:19 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcpcd.c if-bsd.c
if-options.c ipv6.h

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/dhcpcd/dist/src/bpf.c
cvs rdiff -u -r1.23 -r1.24 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/if-bsd.c
cvs rdiff -u -r1.16 -r1.17 src/external/bsd/dhcpcd/dist/src/if-options.c
cvs rdiff -u -r1.3 -r1.4 src/external/bsd/dhcpcd/dist/src/ipv6.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-30 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Tue Jul 30 10:25:03 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcp6.c dhcpcd.c
if-bsd.c if-options.c ipv6.c ipv6.h ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/bpf.c \
src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.22 -r1.23 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.9 -r1.10 src/external/bsd/dhcpcd/dist/src/if-bsd.c \
src/external/bsd/dhcpcd/dist/src/ipv6nd.c
cvs rdiff -u -r1.15 -r1.16 src/external/bsd/dhcpcd/dist/src/if-options.c
cvs rdiff -u -r1.2 -r1.3 src/external/bsd/dhcpcd/dist/src/ipv6.c \
src/external/bsd/dhcpcd/dist/src/ipv6.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-30 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Tue Jul 30 10:25:03 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: bpf.c dhcp.c dhcp6.c dhcpcd.c
if-bsd.c if-options.c ipv6.c ipv6.h ipv6nd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 src/external/bsd/dhcpcd/dist/src/bpf.c \
src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.22 -r1.23 src/external/bsd/dhcpcd/dist/src/dhcp.c \
src/external/bsd/dhcpcd/dist/src/dhcpcd.c
cvs rdiff -u -r1.9 -r1.10 src/external/bsd/dhcpcd/dist/src/if-bsd.c \
src/external/bsd/dhcpcd/dist/src/ipv6nd.c
cvs rdiff -u -r1.15 -r1.16 src/external/bsd/dhcpcd/dist/src/if-options.c
cvs rdiff -u -r1.2 -r1.3 src/external/bsd/dhcpcd/dist/src/ipv6.c \
src/external/bsd/dhcpcd/dist/src/ipv6.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/bpf.c
diff -u src/external/bsd/dhcpcd/dist/src/bpf.c:1.10 src/external/bsd/dhcpcd/dist/src/bpf.c:1.11
--- src/external/bsd/dhcpcd/dist/src/bpf.c:1.10	Wed Jul 24 09:57:43 2019
+++ src/external/bsd/dhcpcd/dist/src/bpf.c	Tue Jul 30 10:25:03 2019
@@ -93,7 +93,7 @@ bpf_frame_header_len(const struct interf
 	}
 }
 
-static const uint8_t etherbroadcastaddr[] =
+static const uint8_t etherbcastaddr[] =
 { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
 int
@@ -104,7 +104,7 @@ bpf_frame_bcast(const struct interface *
 	case ARPHRD_ETHER:
 		return memcmp(frame +
 		offsetof(struct ether_header, ether_dhost),
-		etherbroadcastaddr, sizeof(etherbroadcastaddr));
+		etherbcastaddr, sizeof(etherbcastaddr));
 	default:
 		return -1;
 	}
@@ -552,6 +552,12 @@ bpf_arp(struct interface *ifp, int fd)
 }
 #endif
 
+#define	BPF_M_FHLEN	0
+#define	BPF_M_IPHLEN	1
+#define	BPF_M_IPLEN	2
+#define	BPF_M_UDP	3
+#define	BPF_M_UDPLEN	4
+
 static const struct bpf_insn bpf_bootp_ether[] = {
 	/* Make sure this is an IP packet. */
 	BPF_STMT(BPF_LD + BPF_H + BPF_ABS,
@@ -561,16 +567,26 @@ static const struct bpf_insn bpf_bootp_e
 
 	/* Load frame header length into X. */
 	BPF_STMT(BPF_LDX + BPF_W + BPF_IMM, sizeof(struct ether_header)),
-	/* Copy to M0. */
-	BPF_STMT(BPF_STX, 0),
+	/* Copy frame header length to memory */
+	BPF_STMT(BPF_STX, BPF_M_FHLEN),
 };
 #define BPF_BOOTP_ETHER_LEN	__arraycount(bpf_bootp_ether)
 
 static const struct bpf_insn bpf_bootp_filter[] = {
-	/* Make sure it's an optionless IPv4 packet. */
+	/* Make sure it's an IPv4 packet. */
+	BPF_STMT(BPF_LD + BPF_B + BPF_IND, 0),
+	BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0xf0),
+	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x40, 1, 0),
+	BPF_STMT(BPF_RET + BPF_K, 0),
+
+	/* Ensure IP header length is big enough and
+	 * store the IP header length in memory. */
 	BPF_STMT(BPF_LD + BPF_B + BPF_IND, 0),
-	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0x45, 1, 0),
+	BPF_STMT(BPF_ALU + BPF_AND + BPF_K, 0x0f),
+	BPF_STMT(BPF_ALU + BPF_MUL + BPF_K, 4),
+	BPF_JUMP(BPF_JMP + BPF_JGE + BPF_K, sizeof(struct ip), 1, 0),
 	BPF_STMT(BPF_RET + BPF_K, 0),
+	BPF_STMT(BPF_ST, BPF_M_IPHLEN),
 
 	/* Make sure it's a UDP packet. */
 	BPF_STMT(BPF_LD + BPF_B + BPF_IND, offsetof(struct ip, ip_p)),
@@ -582,39 +598,42 @@ static const struct bpf_insn bpf_bootp_f
 	BPF_JUMP(BPF_JMP + BPF_JSET + BPF_K, 0x1fff, 0, 1),
 	BPF_STMT(BPF_RET + BPF_K, 0),
 
-	/* Store IP location in M1. */
+	/* Store IP length. */
 	BPF_STMT(BPF_LD + BPF_H + BPF_IND, offsetof(struct ip, ip_len)),
-	BPF_STMT(BPF_ST, 1),
-
-	/* Store IP length in M2. */
-	BPF_STMT(BPF_LD + BPF_H + BPF_IND, offsetof(struct ip, ip_len)),
-	BPF_STMT(BPF_ST, 2),
+	BPF_STMT(BPF_ST, BPF_M_IPLEN),
 
 	/* Advance to the UDP header. */
-	BPF_STMT(BPF_MISC + BPF_TXA, 0),
-	BPF_STMT(BPF_ALU + BPF_ADD + BPF_K, sizeof(struct ip)),
+	BPF_STMT(BPF_LD + BPF_MEM, BPF_M_IPHLEN),
+	BPF_STMT(BPF_ALU + BPF_ADD + BPF_X, 0),
 	BPF_STMT(BPF_MISC + BPF_TAX, 0),
 
-	/* Store X in M3. */
-	BPF_STMT(BPF_STX, 3),
+	/* Store UDP location */
+	BPF_STMT(BPF_STX, BPF_M_UDP),
 
 	/* Make sure it's from and to the right port. */
 	BPF_STMT(BPF_LD + BPF_W + BPF_IND, 0),
 	BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, (BOOTPS << 16) + BOOTPC, 1, 0),
 	BPF_STMT(BPF_RET + BPF_K, 0),
 
-	/* Store UDP length in X. */
+	/* Store UDP length. */
 	BPF_STMT(BPF_LD + BPF_H + BPF_IND, offsetof(struct udphdr, uh_ulen)),
+	BPF_STMT(BPF_ST, BPF_M_UDPLEN),
+
+	/* Ensure that UDP length + IP header length == IP length */
+	/* Copy IP header length to X. */
+	BPF_STMT(BPF_LDX + BPF_MEM, BPF_M_IPHLEN),
+	/* Add UDP length (A) to IP header length (X). */
+	BPF_STMT(BPF_ALU + BPF_ADD + BPF_X, 0),
+	/* Store result in X. */
 	BPF_STMT(BPF_MISC + BPF_TAX, 0),
-	/* Copy IP length in M2 to A. */
-	BPF_STMT(BPF_LD + BPF_MEM, 2),
-	/* Ensure IP length - IP header size == UDP length. */
-	BPF_STMT(BPF_ALU + BPF_SUB + BPF_K, sizeof(struct ip)),
+	/* Copy IP length to A. */
+	BPF_STMT(BPF_LD + BPF_MEM, BPF_M_IPLEN),
+	/* Ensure X == A. */
 	BPF_JUMP(BPF_JMP + 

CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:53:45 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp6.c ipv6.c ipv6.h ipv6nd.c

Log Message:
As dhcpcd no longer supports IPv4 address advertisement for SMALL builds,
remove the equivalent IPv6 functionality.
This shouldn't be an issue as this is only used for IPv6 address sharing,
which only the NetBSD kernel currently supports.


To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.1.1.17 -r1.2 src/external/bsd/dhcpcd/dist/src/ipv6.c
cvs rdiff -u -r1.1.1.11 -r1.2 src/external/bsd/dhcpcd/dist/src/ipv6.h
cvs rdiff -u -r1.8 -r1.9 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:53:45 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp6.c ipv6.c ipv6.h ipv6nd.c

Log Message:
As dhcpcd no longer supports IPv4 address advertisement for SMALL builds,
remove the equivalent IPv6 functionality.
This shouldn't be an issue as this is only used for IPv6 address sharing,
which only the NetBSD kernel currently supports.


To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/external/bsd/dhcpcd/dist/src/dhcp6.c
cvs rdiff -u -r1.1.1.17 -r1.2 src/external/bsd/dhcpcd/dist/src/ipv6.c
cvs rdiff -u -r1.1.1.11 -r1.2 src/external/bsd/dhcpcd/dist/src/ipv6.h
cvs rdiff -u -r1.8 -r1.9 src/external/bsd/dhcpcd/dist/src/ipv6nd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp6.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp6.c:1.9 src/external/bsd/dhcpcd/dist/src/dhcp6.c:1.10
--- src/external/bsd/dhcpcd/dist/src/dhcp6.c:1.9	Wed Jul 24 09:57:43 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp6.c	Fri Jul 26 10:53:45 2019
@@ -1523,7 +1523,9 @@ dhcp6_dadcallback(void *arg)
 if (valid)
 	dhcpcd_daemonise(ifp->ctx);
 			}
+#ifdef ND6_ADVERTISE
 			ipv6nd_advertise(ia);
+#endif
 		}
 	}
 }
@@ -3927,16 +3929,20 @@ dhcp6_free(struct interface *ifp)
 void
 dhcp6_abort(struct interface *ifp)
 {
+#ifdef ND6_ADVERTISE
 	struct dhcp6_state *state;
 	struct ipv6_addr *ia;
+#endif
 
 	eloop_timeout_delete(ifp->ctx->eloop, dhcp6_start1, ifp);
+#ifdef ND6_ADVERTISE
 	state = D6_STATE(ifp);
 	if (state == NULL)
 		return;
 	TAILQ_FOREACH(ia, >addrs, next) {
 		ipv6nd_advertise(ia);
 	}
+#endif
 }
 
 void

Index: src/external/bsd/dhcpcd/dist/src/ipv6.c
diff -u src/external/bsd/dhcpcd/dist/src/ipv6.c:1.1.1.17 src/external/bsd/dhcpcd/dist/src/ipv6.c:1.2
--- src/external/bsd/dhcpcd/dist/src/ipv6.c:1.1.1.17	Wed Jul 24 09:54:54 2019
+++ src/external/bsd/dhcpcd/dist/src/ipv6.c	Fri Jul 26 10:53:45 2019
@@ -616,8 +616,10 @@ ipv6_deleteaddr(struct ipv6_addr *ia)
 		}
 	}
 
+#ifdef ND6_ADVERTISE
 	/* Advertise the address if it exists on another interface. */
 	ipv6nd_advertise(ia);
+#endif
 }
 
 static int
@@ -625,8 +627,10 @@ ipv6_addaddr1(struct ipv6_addr *ia, cons
 {
 	struct interface *ifp;
 	uint32_t pltime, vltime;
-	bool vltime_was_zero;
 	__printflike(1, 2) void (*logfunc)(const char *, ...);
+#ifdef ND6_ADVERTISE
+	bool vltime_was_zero;
+#endif
 #ifdef __sun
 	struct ipv6_state *state;
 	struct ipv6_addr *ia2;
@@ -694,7 +698,9 @@ ipv6_addaddr1(struct ipv6_addr *ia, cons
 		" seconds",
 		ifp->name, ia->prefix_pltime, ia->prefix_vltime);
 
+#ifdef ND6_ADVERTISE
 	vltime_was_zero = ia->prefix_vltime == 0;
+#endif
 	if (if_address6(RTM_NEWADDR, ia) == -1) {
 		logerr(__func__);
 		/* Restore real pltime and vltime */
@@ -758,9 +764,11 @@ ipv6_addaddr1(struct ipv6_addr *ia, cons
 	}
 #endif
 
+#ifdef ND6_ADVERTISE
 	/* Re-advertise the preferred address to be safe. */
 	if (!vltime_was_zero)
 		ipv6nd_advertise(ia);
+#endif
 
 	return 0;
 }
@@ -1081,9 +1089,11 @@ ipv6_handleifa(struct dhcpcd_ctx *ctx,
 	case RTM_DELADDR:
 		if (ia != NULL) {
 			TAILQ_REMOVE(>addrs, ia, next);
+#ifdef ND6_ADVERTISE
 			/* Advertise the address if it exists on
 			 * another interface. */
 			ipv6nd_advertise(ia);
+#endif
 			/* We'll free it at the end of the function. */
 		}
 		break;

Index: src/external/bsd/dhcpcd/dist/src/ipv6.h
diff -u src/external/bsd/dhcpcd/dist/src/ipv6.h:1.1.1.11 src/external/bsd/dhcpcd/dist/src/ipv6.h:1.2
--- src/external/bsd/dhcpcd/dist/src/ipv6.h:1.1.1.11	Wed Jul 24 09:54:54 2019
+++ src/external/bsd/dhcpcd/dist/src/ipv6.h	Fri Jul 26 10:53:45 2019
@@ -149,6 +149,10 @@
 #  define IN6_IFF_DETACHED	0
 #endif
 
+#ifndef SMALL
+#  define ND6_ADVERTISE
+#endif
+
 #ifdef INET6
 TAILQ_HEAD(ipv6_addrhead, ipv6_addr);
 struct ipv6_addr {

Index: src/external/bsd/dhcpcd/dist/src/ipv6nd.c
diff -u src/external/bsd/dhcpcd/dist/src/ipv6nd.c:1.8 src/external/bsd/dhcpcd/dist/src/ipv6nd.c:1.9
--- src/external/bsd/dhcpcd/dist/src/ipv6nd.c:1.8	Wed Jul 24 09:57:43 2019
+++ src/external/bsd/dhcpcd/dist/src/ipv6nd.c	Fri Jul 26 10:53:45 2019
@@ -389,6 +389,7 @@ ipv6nd_sendrsprobe(void *arg)
 	}
 }
 
+#ifdef ND6_ADVERTISE
 static void
 ipv6nd_sendadvertisement(void *arg)
 {
@@ -526,6 +527,7 @@ ipv6nd_advertise(struct ipv6_addr *ia)
 	eloop_timeout_delete(ctx->eloop, ipv6nd_sendadvertisement, iaf);
 	ipv6nd_sendadvertisement(iaf);
 }
+#endif /* ND6_ADVERTISE */
 
 static void
 ipv6nd_expire(void *arg)
@@ -908,7 +910,9 @@ try_script:
 	return;
 			}
 		}
+#ifdef ND6_ADVERTISE
 		ipv6nd_advertise(ia);
+#endif
 	}
 }
 



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:47:29 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
Allow dhcpcd to be built without ARP support for SMALL builds.
This is fine because the kernel supports RFC 5227 which dhcpcd uses
instead to detect and act on address duplication.


To generate a diff of this commit:
cvs rdiff -u -r1.21 -r1.22 src/external/bsd/dhcpcd/dist/src/dhcp.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:47:29 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c

Log Message:
Allow dhcpcd to be built without ARP support for SMALL builds.
This is fine because the kernel supports RFC 5227 which dhcpcd uses
instead to detect and act on address duplication.


To generate a diff of this commit:
cvs rdiff -u -r1.21 -r1.22 src/external/bsd/dhcpcd/dist/src/dhcp.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.21 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.22
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.21	Thu Jul 25 08:55:18 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Fri Jul 26 10:47:29 2019
@@ -2329,7 +2329,9 @@ dhcp_arp_new(struct interface *ifp, stru
 	return astate;
 }
 #endif
+#endif /* ARP */
 
+#if defined(ARP) || defined(KERNEL_RFC5227)
 static int
 dhcp_arp_address(struct interface *ifp)
 {
@@ -2417,7 +2419,7 @@ dhcp_static(struct interface *ifp)
 	ia ? >addr : >req_addr,
 	ia ? >mask : >req_mask);
 	if (state->offer_len)
-#ifdef ARP
+#if defined(ARP) || defined(KERNEL_RFC5227)
 		dhcp_arp_bind(ifp);
 #else
 		dhcp_bind(ifp);
@@ -3210,7 +3212,7 @@ rapidcommit:
 	lease->frominfo = 0;
 	eloop_timeout_delete(ifp->ctx->eloop, NULL, ifp);
 
-#ifdef ARP
+#if defined(ARP) || defined(KERNEL_RFC5227)
 	dhcp_arp_bind(ifp);
 #else
 	dhcp_bind(ifp);



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:39:29 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcpcd.8.in

Log Message:
Replace hook example 10-wpa_supplicant with 29-lookup-hostname.
Fixes PR install/54351.


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.13 -r1.2 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in
diff -u src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in:1.1.1.13 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in:1.2
--- src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in:1.1.1.13	Wed Jul 24 09:54:51 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in	Fri Jul 26 10:39:29 2019
@@ -24,7 +24,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd July 23, 2019
+.Dd July 25, 2019
 .Dt DHCPCD 8
 .Os
 .Sh NAME
@@ -234,12 +234,11 @@ and need to be copied to
 .Pa @HOOKDIR@
 if you intend to use them.
 For example, you could install
-.Pa 10-wpa_supplicant
+.Pa 29-lookup-hostname
 so that
 .Nm
-can ensure that
-.Xr wpa_supplicant 8
-is always running on a hot-plugged wireless interface.
+can lookup the hostname of the IP address in DNS if no hostname
+is given by the lease and one is not already set.
 .Ss Fine tuning
 You can fine-tune the behaviour of
 .Nm
@@ -404,10 +403,6 @@ is specified then this applies to all in
 If
 .Nm
 is not running, then it starts up as normal.
-This may also cause
-.Xr wpa_supplicant 8
-to reload its configuration for each interface as well if the
-relevant hook script has been installed.
 .It Fl N , Fl Fl renew Op Ar interface
 Notifies
 .Nm



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-26 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Fri Jul 26 10:39:29 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcpcd.8.in

Log Message:
Replace hook example 10-wpa_supplicant with 29-lookup-hostname.
Fixes PR install/54351.


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.13 -r1.2 src/external/bsd/dhcpcd/dist/src/dhcpcd.8.in

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-25 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Thu Jul 25 08:55:18 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcpcd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.20 -r1.21 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.21 -r1.22 src/external/bsd/dhcpcd/dist/src/dhcpcd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/dhcp.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcp.c:1.20 src/external/bsd/dhcpcd/dist/src/dhcp.c:1.21
--- src/external/bsd/dhcpcd/dist/src/dhcp.c:1.20	Wed Jul 24 09:57:43 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcp.c	Thu Jul 25 08:55:18 2019
@@ -127,7 +127,7 @@ static const char * const dhcp_params[] 
 
 static int dhcp_openbpf(struct interface *);
 static void dhcp_start1(void *);
-#ifdef ARP
+#if defined(ARP) && (!defined(KERNEL_RFC5227) || defined(ARPING))
 static void dhcp_arp_found(struct arp_state *, const struct arp_msg *);
 #endif
 static void dhcp_handledhcp(struct interface *, struct bootp *, size_t,
@@ -1958,6 +1958,7 @@ dhcp_rebind(void *arg)
 	send_rebind(ifp);
 }
 
+#if defined(ARP) || defined(IN_IFF_DUPLICATED)
 static void
 dhcp_finish_dad(struct interface *ifp, struct in_addr *ia)
 {
@@ -2026,19 +2027,22 @@ dhcp_addr_duplicated(struct interface *i
 	eloop_timeout_add_sec(ifp->ctx->eloop,
 	DHCP_RAND_MAX, dhcp_discover, ifp);
 }
+#endif
 
-#ifdef ARP
+#if defined(ARP) && (!defined(KERNEL_RFC5227) || defined(ARPING))
 static void
 dhcp_arp_not_found(struct arp_state *astate)
 {
 	struct interface *ifp;
+#ifdef ARPING
 	struct dhcp_state *state;
 	struct if_options *ifo;
+#endif
 
 	ifp = astate->iface;
+#ifdef ARPING
 	state = D_STATE(ifp);
 	ifo = ifp->options;
-#ifdef ARPING
 	if (ifo->arping_len && state->arping_index < ifo->arping_len) {
 		/* We didn't find a profile for this
 		 * address or hwaddr, so move to the next
@@ -2062,12 +2066,11 @@ static void
 dhcp_arp_found(struct arp_state *astate, const struct arp_msg *amsg)
 {
 	struct in_addr addr;
+	struct interface *ifp = astate->iface;
 #ifdef ARPING
-	struct interface *ifp;
 	struct dhcp_state *state;
 	struct if_options *ifo;
 
-	ifp = astate->iface;
 	state = D_STATE(ifp);
 
 	ifo = ifp->options;
@@ -2093,6 +2096,8 @@ dhcp_arp_found(struct arp_state *astate,
 		dhcpcd_startinterface(ifp);
 		return;
 	}
+#else
+	UNUSED(amsg);
 #endif
 
 	addr = astate->addr;
@@ -2304,6 +2309,7 @@ dhcp_arp_defend_failed(struct arp_state 
 }
 #endif
 
+#if !defined(KERNEL_RFC5227) || defined(ARPING)
 static struct arp_state *
 dhcp_arp_new(struct interface *ifp, struct in_addr *addr)
 {
@@ -2322,6 +2328,7 @@ dhcp_arp_new(struct interface *ifp, stru
 #endif
 	return astate;
 }
+#endif
 
 static int
 dhcp_arp_address(struct interface *ifp)

Index: src/external/bsd/dhcpcd/dist/src/dhcpcd.c
diff -u src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.21 src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.22
--- src/external/bsd/dhcpcd/dist/src/dhcpcd.c:1.21	Wed Jul 24 09:57:43 2019
+++ src/external/bsd/dhcpcd/dist/src/dhcpcd.c	Thu Jul 25 08:55:18 2019
@@ -1210,9 +1210,11 @@ dhcpcd_handlehwaddr(struct dhcpcd_ctx *c
 static void
 if_reboot(struct interface *ifp, int argc, char **argv)
 {
+#ifdef INET
 	unsigned long long oldopts;
 
 	oldopts = ifp->options->options;
+#endif
 	script_runreason(ifp, "RECONFIGURE");
 	dhcpcd_initstate1(ifp, argc, argv, 0);
 #ifdef INET
@@ -2123,6 +2125,12 @@ exit1:
 		}
 		free(ctx.ifaces);
 	}
+#ifdef HAVE_OPEN_MEMSTREAM
+	if (ctx.script_fp)
+		fclose(ctx.script_fp);
+#endif
+	free(ctx.script_buf);
+	free(ctx.script_env);
 	free_options(, ifo);
 	rt_dispose();
 	free(ctx.duid);
@@ -2146,11 +2154,5 @@ exit1:
 	if (ctx.options & DHCPCD_FORKED)
 		_exit(i); /* so atexit won't remove our pidfile */
 #endif
-#ifdef HAVE_OPEN_MEMSTREAM
-	if (ctx.script_fp)
-		fclose(ctx.script_fp);
-#endif
-	free(ctx.script_buf);
-	free(ctx.script_env);
 	return i;
 }



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-25 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Thu Jul 25 08:55:18 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: dhcp.c dhcpcd.c

Log Message:
Sync


To generate a diff of this commit:
cvs rdiff -u -r1.20 -r1.21 src/external/bsd/dhcpcd/dist/src/dhcp.c
cvs rdiff -u -r1.21 -r1.22 src/external/bsd/dhcpcd/dist/src/dhcpcd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-24 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Jul 24 15:06:21 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: logerr.c

Log Message:
Fix SMALL build.


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.4 -r1.2 src/external/bsd/dhcpcd/dist/src/logerr.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/dhcpcd/dist/src/logerr.c
diff -u src/external/bsd/dhcpcd/dist/src/logerr.c:1.1.1.4 src/external/bsd/dhcpcd/dist/src/logerr.c:1.2
--- src/external/bsd/dhcpcd/dist/src/logerr.c:1.1.1.4	Wed Jul 24 09:54:55 2019
+++ src/external/bsd/dhcpcd/dist/src/logerr.c	Wed Jul 24 15:06:21 2019
@@ -209,6 +209,7 @@ vlogmessage(int pri, const char *fmt, va
 
 #ifdef SMALL
 	vsyslog(pri, fmt, args);
+	return len;
 #else
 	if (ctx->log_file == NULL) {
 		vsyslog(pri, fmt, args);



CVS commit: src/external/bsd/dhcpcd/dist/src

2019-07-24 Thread Roy Marples
Module Name:src
Committed By:   roy
Date:   Wed Jul 24 15:06:21 UTC 2019

Modified Files:
src/external/bsd/dhcpcd/dist/src: logerr.c

Log Message:
Fix SMALL build.


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.4 -r1.2 src/external/bsd/dhcpcd/dist/src/logerr.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-04 Thread Kamil Rytarowski
On 04.08.2018 05:44, Robert Elz wrote:
> Date:Sat, 4 Aug 2018 04:24:01 +0200
> From:Kamil Rytarowski 
> Message-ID:  <568544f4-36d5-853e-cdf8-248f84fad...@gmx.com>
> 
>   | I haven't changed any optimization or similar flags for the builds.
> 
> Then why did the ssh example start giving "perhaps used uninit" warnings
> when it had not before?Something changed.

I can refer to the opinion of Joerg (expressed on the NetBSD mailing
lists) that reporting uninitiated variables are one of the biggest bugs
in GCC.

I will leave tracking it down to someone with knowledge of GCC code
generation algorithms, and push a patch to OpenSSH.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Sat, 4 Aug 2018 04:24:01 +0200
From:Kamil Rytarowski 
Message-ID:  <568544f4-36d5-853e-cdf8-248f84fad...@gmx.com>

  | I haven't changed any optimization or similar flags for the builds.

Then why did the ssh example start giving "perhaps used uninit" warnings
when it had not before?Something changed.

  | Freshly crashed pmax kernel due to integer overflow is a kernel (or
  | virtualization) bug, but it's also a definition of UB, that it can crash
  | the computer.

I don't know the circumstances of that one, but sure, overflow can cause
all kinds of problems, and if it actually occurs, almost anything is possible,
depending upon just what the code is doing.   I'm not sure what your point
is here, no-one is suggesting that real bugs not get fixed.

  | The dhcpcd one was fixed first in upstream dhcpcd before landing it to src/.

"fixed" is the wrong word, as there neither is, nor ever was, any bug there
to "fix".   You apparently silenced a bogus sanitiser induced warning.   That
is not a "fix".

kre



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 04.08.2018 03:23, Robert Elz wrote:
> Date:Sat, 4 Aug 2018 02:15:15 +0200
> From:Kamil Rytarowski 
> Message-ID:  
> 
> 
>   | In general there shall not be a relation between -O level and
>   | sanitizers. Sanitizers do not need -O0  or -g for operation.
> 
> That's fine.   But are you doing compiles that way?  (necessary or not)
> 

I haven't changed any optimization or similar flags for the builds.

>   | GCC also enables more warnings for UBSan that have to be addressed in
>   | order to compile the source, as the code would be UB anyway (like
>   | changing the signedness bit with a shift operation).
> 
> Sure, some of those, even though they're not really problems, are easy
> to fix in a totally harmless way.   That's fine.   The UB is a technical C
> issue, not really anything that ever fails in those cases though.
> 

Freshly crashed pmax kernel due to integer overflow is a kernel (or
virtualization) bug, but it's also a definition of UB, that it can crash
the computer.

>   | And regarding utility of the Undefined Behavior Sanitizer and coverage
>   | of new tests.. we have just caught a bug on pmax that an integer
>   | overflow crashed the kernel:
> 
> Sure, no-one is saying that the extra work is not worth while.   Just that
> you are sometimes fixing non-problems (and causing code churn to do
> it - particularly when the code being changed comes from upstream ... in
> the dhcpcd case that is not such an issue, as if needed, Roy will fix that
> as well, but why would anyone expect the openbsd people to alter ssh
> to fix a non problem ?)
> 

Some Undefined Behavior fixes were pulled by FreeBSD (at least one of
them merged McKusick!). Regarding the OpenSSH case, I'm in touch with
some of their developers and concluded to push it to one of their
mailinglist. One of the UB patches was already merged by the OpenBSD
project (in tmux).

I will go the same way with other patches and submit them upstream, even
if some are cautious. The dhcpcd one was fixed first in upstream dhcpcd
before landing it to src/.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Sat, 4 Aug 2018 02:15:15 +0200
From:Kamil Rytarowski 
Message-ID:  


  | In general there shall not be a relation between -O level and
  | sanitizers. Sanitizers do not need -O0  or -g for operation.

That's fine.   But are you doing compiles that way?  (necessary or not)

  | GCC is known for reporting uninitialized variables and I wouldn't blame
  | sanitizers for it.

I wasn't.   The one (which isn't) in the ssh code has been there for
ages, and has compiled successfully, for ages   There has to be a
reason why it only needed action yesterday.

  | We just initialize them to tune it down and this is the current practice.

Yes, I know - some of them are real potential problems, even if the
circumstances that lead to the problem are so unlikely that we never
see them in real life - others take knowledge about the environment
the compiler does not have, or just require flow analysis more
complex than it is reasonable to expect of the compiler, in order to know
that there is not actually a problem.The real bugs we fix, obviously.
The ones the compiler cannot detect are not bugs we deal with as you
said.   But when the compiler is able to detect there is no problem, but
we are simply preventing it doing so, we fix the way we use the compiler.

  | GCC also enables more warnings for UBSan that have to be addressed in
  | order to compile the source, as the code would be UB anyway (like
  | changing the signedness bit with a shift operation).

Sure, some of those, even though they're not really problems, are easy
to fix in a totally harmless way.   That's fine.   The UB is a technical C
issue, not really anything that ever fails in those cases though.

There has (is currently) an issue with posix, where a current bug resulution
requires abs(INT_MIN) to be INT_MIN (ie: abs(n) can end up < 0).   In C
that's undefined.   POSIX requires 2's complement however (unlike C) and
can rely upon what happens with 0 - INT_MAX even if C says that is
undefined.Some people like it, others do not...  (what the end result
will be I have no idea.)

  | I don't agree with strong opinions against cautious warnings/errors from
  | a compiler.

Sure, but when the warning goes off, we need to analyse the issue and
see what the actual problem is, not just blindly do whatever makes the
compiler stop issuing the warning.

  |They are there for purpose and dhcpcd could be really broken
  | with the same code, but with a different context.

No, it could not.   The only possible issue was if the packet was
invalid (too short a len) but that was not what the warning was
about (and could not have been, as there was nothing undefined
if that happened, just an unwanted result).

  | And regarding utility of the Undefined Behavior Sanitizer and coverage
  | of new tests.. we have just caught a bug on pmax that an integer
  | overflow crashed the kernel:

Sure, no-one is saying that the extra work is not worth while.   Just that
you are sometimes fixing non-problems (and causing code churn to do
it - particularly when the code being changed comes from upstream ... in
the dhcpcd case that is not such an issue, as if needed, Roy will fix that
as well, but why would anyone expect the openbsd people to alter ssh
to fix a non problem ?)

Once again, please do not change code to fix gcc warnings unless you
get the same warning with the code compiled with -O2 (or more).

kre



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 04.08.2018 01:31, Robert Elz wrote:
> Kamil: assuming you agree that this is a reasonable analysis, I'd suggest
> no more code changes based upon gcc warnings issued this way.

In general there shall not be a relation between -O level and
sanitizers. Sanitizers do not need -O0  or -g for operation. UBSan does
not need disabled optimization for reporting issues in exact location in
the code. It also does not need debug information (DWARF or similar)...
however a runtime might make use of the additional data to print more
verbose messages or stacktraces.

GCC is known for reporting uninitialized variables and I wouldn't blame
sanitizers for it. We just initialize them to tune it down and this is
the current practice.

GCC also enables more warnings for UBSan that have to be addressed in
order to compile the source, as the code would be UB anyway (like
changing the signedness bit with a shift operation).

I don't agree with strong opinions against cautious warnings/errors from
a compiler. They are there for purpose and dhcpcd could be really broken
with the same code, but with a different context.

And regarding utility of the Undefined Behavior Sanitizer and coverage
of new tests.. we have just caught a bug on pmax that an integer
overflow crashed the kernel:

UB caused to crash pmax.. divrem_overflow_signed_div: pexpect reported
EOF - VMM exited unexpectedly



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Valery Ushakov
On Fri, Aug 03, 2018 at 18:08:24 +0300, Valery Ushakov wrote:

> On Fri, Aug 03, 2018 at 15:54:24 +0200, Martin Husemann wrote:
> 
> > On Fri, Aug 03, 2018 at 08:28:55PM +0700, Robert Elz wrote:
> > > Where is the signed arithmetic that was supposedly a probem?
> > 
> > Ah, stupid C integer promotion rules.  uint16_t is promoted to int
> > here, not unsigned int or size_t.
> 
> Hmm, i don't think that's true.

Nah, you are right.  "THEN" is the important word here.

But as it transpired in another branch of this thread the problem was
something entirely different...

>   6.3.1.8  Usual arithmetic conversions
> 
>   ...
> Otherwise, the integer promotions are performed on both
> operands.   Then the following rules are applied to the
> promoted operands:

-uwe


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread maya
On Fri, Aug 03, 2018 at 07:46:01PM +0100, Roy Marples wrote:
> We could split the term, but merely storing the result of htons in it's own
> variable creates a larger binary for no good reason as i see it.
> 

I suspect that the compiler will generate the same code anyway when
using a local variable for intermediate results, feel free to write
the cost as you find most legible :-)


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Fri, 3 Aug 2018 19:46:01 +0100
From:Roy Marples 
Message-ID:  

  | Considering both methods work and the result of htons is a uint16_t (but 
  | is this always guaranteed?)

ntohs() (not that it matters) and "always" is a big word, but that is how
it is defined by POSIX, so it should be something that we can rely upon.

  | is this just an over-zealous compiler warning?

Not so much over zealous, as just plain lazy... (given Kamil's most
recent message):

  | conversion to 'long unsigned int' from 'int' may change the sign of the
  | result [-Werror=sign-conversion]
  |   *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
  |  ^

That's not the overflow on subtract that you said before, and for this one
I can see how the cast can help ... but that's just sheer laziness on the part
of the compiler.

The "int" which is there was created by the compiler, it knows (or should know)
that the underlying value is in the rangs [0..65535] and cannot possibly have
its sign changed when it is converted to long unsigned int.

It would be more understandable if the int appeared somewhere earlier, but
here this is all in this one expression, one type promotion on top of another.

Get the idiot compiler fixed, and then remove the cast.   In the meantime,
at least mark it with a comment indicating that the cast should not be needed,
and is there purely to appease gcc.

kre



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 03.08.2018 20:49, Roy Marples wrote:
> On 03/08/2018 15:22, Robert Elz wrote:
>> Whether there need to be any attention to the possibility
>> of a malformed packet I will leave for Roy to decide (I am
>> assuming probably not) but that added cast just looks to be
>> a bandaid for a broken compiler (sanitiser).
> 
> The contents are verified further up the stack.
> I'm inclined to agree it's a dodgy compiler warning, but I'm not really
> an expert here.
> 
> Roy

I've repeated the compiler error as I forgot the exact error message (it
was fixed in a local branch a while ago):

/public/src.git/external/bsd/dhcpcd/dist/src/dhcp.c: In function
'get_udp_data':
/public/src.git/external/bsd/dhcpcd/dist/src/dhcp.c:3270:29: error:
conversion to 'long unsigned int' from 'int' may change the sign of the
result [-Werror=sign-conversion]
  *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
 ^


http://netbsd.org/~kamil/patch-00068-dhcpcd-get_udp_data.txt



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Roy Marples

On 03/08/2018 15:22, Robert Elz wrote:

Whether there need to be any attention to the possibility
of a malformed packet I will leave for Roy to decide (I am
assuming probably not) but that added cast just looks to be
a bandaid for a broken compiler (sanitiser).


The contents are verified further up the stack.
I'm inclined to agree it's a dodgy compiler warning, but I'm not really 
an expert here.


Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Roy Marples

On 03/08/2018 14:02, Martin Husemann wrote:

On Fri, Aug 03, 2018 at 02:47:53PM +0200, Kamil Rytarowski wrote:

Further if there ever was a potential problem from this line ...

*len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
then
*len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);



It was a build time error generated by GCC. The compiler detected that
both sizeof() could be large enough to overflow the result returned from
  ntohs(3). And overflowing a signed integer is Undefined Behavior.


But we do not do this here.


This change points to the compiler that the code is safe.


What exactly makes the code safe now? If ntohs(p->ip.ip_len) <
(sizeof(p->ip) + sizeof(p->udp)) then we are now in even more serious
trouble.

Does splitting the term help?

uint16_t hdr_size = sizeof(p->ip) - sizeof(p->udp);
uint16_t pkt_size = ntohs(p->ip.ip_len);
KASSERT(pkt_size > hdr_size);
*len = pkt_size > hdr_size ? pkt_size-hdr_size : 0;

or something like that?


We could split the term, but merely storing the result of htons in it's 
own variable creates a larger binary for no good reason as i see it.


Considering both methods work and the result of htons is a uint16_t (but 
is this always guaranteed?) is this just an over-zealous compiler warning?


Roy


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Valery Ushakov
On Fri, Aug 03, 2018 at 15:54:24 +0200, Martin Husemann wrote:

> On Fri, Aug 03, 2018 at 08:28:55PM +0700, Robert Elz wrote:
> > Where is the signed arithmetic that was supposedly a probem?
> 
> Ah, stupid C integer promotion rules.  uint16_t is promoted to int
> here, not unsigned int or size_t.

Hmm, i don't think that's true.

  6.3.1.8  Usual arithmetic conversions

  ...
Otherwise, the integer promotions are performed on both
operands.   Then the following rules are applied to the
promoted operands:

 If both operands  have  the  same  type,  then  no
 further conversion is needed.

 Otherwise,  if  both  operands have signed integer
 types or both have  unsigned  integer  types,  the
 operand with the type of lesser integer conversion
 rank is converted to the type of the operand  with
 greater rank.

ntohs returns unsigned uint16_t, sizeof returns unsigned size_t, so
uint16_t, that has "lesser integer convertion rank" (i.e. it's
smaller) should be converted to size_t automagically.

-uwe


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Fri, 3 Aug 2018 15:54:24 +0200
From:Martin Husemann 
Message-ID:  <20180803135424.gc23...@mail.duskware.de>

  | Ah, stupid C integer promotion rules. uint16_t is promoted to int
  | here, not unsigned int or size_t.

Even with that, there should be no problem, in

signed - unsigned

the '-' should be an unsigned - and the result should
be unsigned.   There is no signed arithmetic being done
here to cause an undefined result.

That's the same rule that makes

strlen(s) + 1

be a size_t rather than a ssize_t or whatever.   Otherwise we'd
need to be adding casts to every operation like that, just in case
strlen(s) == MAX_INT and the " +1 " would cause overflow, and
undefined operation.No thanks.

Whether there need to be any attention to the possibility
of a malformed packet I will leave for Roy to decide (I am
assuming probably not) but that added cast just looks to be
a bandaid for a broken compiler (sanitiser).

kre



Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Martin Husemann
On Fri, Aug 03, 2018 at 08:28:55PM +0700, Robert Elz wrote:
> Where is the signed arithmetic that was supposedly a probem?

Ah, stupid C integer promotion rules. uint16_t is promoted to int
here, not unsigned int or size_t. The cast makes all operands the same
type and no promotion happens.

Martin


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Fri, 3 Aug 2018 15:02:28 +0200
From:Martin Husemann 
Message-ID:  <20180803130227.ga23...@mail.duskware.de>

  | What exactly makes the code safe now? If ntohs(p->ip.ip_len) <
  | (sizeof(p->ip) + sizeof(p->udp)) then we are now in even more serious
  | trouble.

Actually, not more serious, the same serious as before.   If adding that
cast change anything at all, the compiler isn't working as it should.

If the values haven't been verieied, they should be.   If they have been
verified, there is no problem and nothing needs fixing (except possibly the
santiizer).

In a later message ...

  | Overflow (underflow) of an unsigned value is defined and GCC stops
  | deducing whether there might be a problem.

But it always was unsigned, ntohs() returns an unsigned result.   Further
even if it was signed, doesn't combining a signed value and an unsigned
one with an arithmetic op result in an unsigned operation?

Where is the signed arithmetic that was supposedly a probem?

kre




Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 03.08.2018 15:20, Martin Husemann wrote:
> On Fri, Aug 03, 2018 at 03:18:18PM +0200, Kamil Rytarowski wrote:
>> The change was indicating to the compiler that code is safe, without
>> changing the algorithm.
> 
> I don't get why.
> 
> Martin
> 

Overflow (underflow) of an unsigned value is defined and GCC stops
deducing whether there might be a problem.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Martin Husemann
On Fri, Aug 03, 2018 at 03:18:18PM +0200, Kamil Rytarowski wrote:
> The change was indicating to the compiler that code is safe, without
> changing the algorithm.

I don't get why.

Martin


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 03.08.2018 15:02, Martin Husemann wrote:
> On Fri, Aug 03, 2018 at 02:47:53PM +0200, Kamil Rytarowski wrote:
>>> Further if there ever was a potential problem from this line ...
>>>
>>> *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
>>> then
>>> *len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
> 
>> It was a build time error generated by GCC. The compiler detected that
>> both sizeof() could be large enough to overflow the result returned from
>>  ntohs(3). And overflowing a signed integer is Undefined Behavior.
> 
> But we do not do this here.
> 
>> This change points to the compiler that the code is safe.
> 
> What exactly makes the code safe now? If ntohs(p->ip.ip_len) <
> (sizeof(p->ip) + sizeof(p->udp)) then we are now in even more serious
> trouble.
> 

The change was indicating to the compiler that code is safe, without
changing the algorithm.

> Does splitting the term help?
> 
>   uint16_t hdr_size = sizeof(p->ip) - sizeof(p->udp);

+

>   uint16_t pkt_size = ntohs(p->ip.ip_len);
>   KASSERT(pkt_size > hdr_size);
>   *len = pkt_size > hdr_size ? pkt_size-hdr_size : 0;
> 
> or something like that?
> 

This looks like a safer approach, but I will defer it to Roy to decide
what to do.

Previously we have agreed with the (size_t) case.

> Martin
> 




signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Martin Husemann
On Fri, Aug 03, 2018 at 02:47:53PM +0200, Kamil Rytarowski wrote:
> > Further if there ever was a potential problem from this line ...
> > 
> > *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
> > then
> > *len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);

> It was a build time error generated by GCC. The compiler detected that
> both sizeof() could be large enough to overflow the result returned from
>  ntohs(3). And overflowing a signed integer is Undefined Behavior.

But we do not do this here.

> This change points to the compiler that the code is safe.

What exactly makes the code safe now? If ntohs(p->ip.ip_len) <
(sizeof(p->ip) + sizeof(p->udp)) then we are now in even more serious
trouble.

Does splitting the term help?

uint16_t hdr_size = sizeof(p->ip) - sizeof(p->udp);
uint16_t pkt_size = ntohs(p->ip.ip_len);
KASSERT(pkt_size > hdr_size);
*len = pkt_size > hdr_size ? pkt_size-hdr_size : 0;

or something like that?

Martin


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Kamil Rytarowski
On 03.08.2018 12:26, Robert Elz wrote:
> Date:Fri, 3 Aug 2018 02:17:33 +
> From:"Kamil Rytarowski" 
> Message-ID:  <20180803021733.b2002f...@cvs.netbsd.org>
> 
>   | GCC with -fsanitize=undefiend detects a potential overflow in the code.
>   | Cast the return value of ntohs(3) to size_t.
> 
> I don't understand the point of this change, and I believe it to
> be wrong and misguided.
> 
> Further if there ever was a potential problem from this line ...
> 
>   *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
> then
>   *len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
> 
> will not fix it.   The possible problem being if ip_len < 28 (the sum of the
> two sizeof's).While I didn't check this code, that possibility is usually
> verified elsewhere, and even if not, adding the cast changes nothing, the
> result is still bogus (that is, at best, the change could be masking a real
> bug, though that's unlikely.)
> 
> This looks to be (somehow) determining that the result of ntohs(ip_len)
> might somehow be negative (or something, I'm not really sure what is
> being believed is happening) but the ntohs() result is (in all cases here)
> a uint16_t (either because the result is literally the arg, which is that 
> type,
> or because it is the result of bswap16() which is declared that way.)
> 
> The sizeof() ops make the expression at least a size_t - so unless size_t
> and uint16_t are the same (which might happen on a pdp-11 or similar,
> though even there it is unlikely I suspect) the narrower one needs to be
> promoted - whether the ntohs() result is implicitly promoted to size_t, or
> explicitly cast cannot make any difference, can it?
> 
> So what exactly is being fixed here?  Which behaviour is supposedly undefined?
> 

It was a build time error generated by GCC. The compiler detected that
both sizeof() could be large enough to overflow the result returned from
 ntohs(3). And overflowing a signed integer is Undefined Behavior.

This change points to the compiler that the code is safe.

> kre
> 




signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2018-08-03 Thread Robert Elz
Date:Fri, 3 Aug 2018 02:17:33 +
From:"Kamil Rytarowski" 
Message-ID:  <20180803021733.b2002f...@cvs.netbsd.org>

  | GCC with -fsanitize=undefiend detects a potential overflow in the code.
  | Cast the return value of ntohs(3) to size_t.

I don't understand the point of this change, and I believe it to
be wrong and misguided.

Further if there ever was a potential problem from this line ...

*len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
then
*len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);

will not fix it.   The possible problem being if ip_len < 28 (the sum of the
two sizeof's).While I didn't check this code, that possibility is usually
verified elsewhere, and even if not, adding the cast changes nothing, the
result is still bogus (that is, at best, the change could be masking a real
bug, though that's unlikely.)

This looks to be (somehow) determining that the result of ntohs(ip_len)
might somehow be negative (or something, I'm not really sure what is
being believed is happening) but the ntohs() result is (in all cases here)
a uint16_t (either because the result is literally the arg, which is that type,
or because it is the result of bswap16() which is declared that way.)

The sizeof() ops make the expression at least a size_t - so unless size_t
and uint16_t are the same (which might happen on a pdp-11 or similar,
though even there it is unlikely I suspect) the narrower one needs to be
promoted - whether the ntohs() result is implicitly promoted to size_t, or
explicitly cast cannot make any difference, can it?

So what exactly is being fixed here?  Which behaviour is supposedly undefined?

kre