Loops depending on undefined behavior

[Michael McConville]

I've discussed the below with otto@, who thought it was sound but wanted
confirmation before I commit.

While debugging rarpd(8) for unrelated reasons, I noticed a loop of the

for (i = 1; i; i <<= 1)

where i is an int.

This relies on shifting into and then out of the sign bit, both of which
are undefined operations. The loop contains no breaks or returns, so
they are always executed.

It was copied around a few times, so there are a few nearly identical
instances included in the below diff.

These instances use i for:

o boolean tests
o comparison with small positive constants (RTA_*)
o &ing with an int

The third case seems questionable. However, (IIUC) integer conversion
rules dictate that the int is implicitly cast to an unsigned int[1] and
that two's complement arithmetic is preserved in this cast.[2]

C integer type rules are such a joy to work with...

Am I interpreting this correctly?

[1] Usual Arithmetic Conversions #3 in:
https://www.securecoding.cert.org/confluence/x/QgE

[2] C99/C11 6.3.1.3
http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1570.pdf


Index: sbin/dhclient/dhclient.c
===================================================================
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.370
diff -u -p -r1.370 dhclient.c
--- sbin/dhclient/dhclient.c 12 Dec 2015 14:48:17 -0000 1.370
+++ sbin/dhclient/dhclient.c 21 Jan 2016 02:16:17 -0000
@@ -160,7 +160,7 @@ int
findproto(char *cp, int n)
{
struct sockaddr *sa;
- int i;
+ unsigned int i;

if (n == 0)
return -1;
Index: usr.sbin/arp/arp.c
===================================================================
RCS file: /cvs/src/usr.sbin/arp/arp.c,v
retrieving revision 1.70
diff -u -p -r1.70 arp.c
--- usr.sbin/arp/arp.c 8 Dec 2015 14:20:24 -0000 1.70
+++ usr.sbin/arp/arp.c 21 Jan 2016 02:16:17 -0000
@@ -695,7 +695,7 @@ rtget(struct sockaddr_inarp **sinp, stru
struct sockaddr_dl *sdl = NULL;
struct sockaddr *sa;
char *cp;
- int i;
+ unsigned int i;

if (rtmsg(RTM_GET) < 0)
return (1);
Index: usr.sbin/ndp/ndp.c
===================================================================
RCS file: /cvs/src/usr.sbin/ndp/ndp.c,v
retrieving revision 1.68
diff -u -p -r1.68 ndp.c
--- usr.sbin/ndp/ndp.c 12 Jan 2016 09:47:13 -0000 1.68
+++ usr.sbin/ndp/ndp.c 21 Jan 2016 02:16:17 -0000
@@ -879,7 +879,7 @@ rtget(struct sockaddr_in6 **sinp, struct
struct sockaddr_dl *sdl = NULL;
struct sockaddr *sa;
char *cp;
- int i;
+ unsigned int i;

if (rtmsg(RTM_GET) < 0)
return (1);
Index: usr.sbin/rarpd/arptab.c
===================================================================
RCS file: /cvs/src/usr.sbin/rarpd/arptab.c,v
retrieving revision 1.25
diff -u -p -r1.25 arptab.c
--- usr.sbin/rarpd/arptab.c 19 Nov 2015 19:31:20 -0000 1.25
+++ usr.sbin/rarpd/arptab.c 21 Jan 2016 02:16:17 -0000
@@ -240,7 +240,7 @@ rtget(struct sockaddr_inarp **sinp, stru
struct sockaddr_dl *sdl = NULL;
struct sockaddr *sa;
char *cp;
- int i;
+ unsigned int i;

if (rtmsg(RTM_GET) < 0)
return (1);

[Todd C. Miller]

Post by Michael McConville
The third case seems questionable. However, (IIUC) integer conversion
rules dictate that the int is implicitly cast to an unsigned int[1] and
that two's complement arithmetic is preserved in this cast.[2]
C integer type rules are such a joy to work with...
Am I interpreting this correctly?

Yes, you are correct. The diff looks fine but there is another instance
in dhclient.c in get_ifa(). OK millert@ with that added.

- todd