wip: Build with -Wconversionlukeshu/safe-conversion

I think this found a real bug in the dhcp packet parser.

I don't think anything called lib9p_str{,n}() values that could be big
enough, but their bounds-checking was broken.

5 files changed, 50 insertions, 37 deletions

diff --git a/libhw_cr/host_util.h b/libhw_cr/host_util.h
index 02c04dc..3d56e97 100644
--- a/libhw_cr/host_util.h
+++ b/libhw_cr/host_util.h
@@ -19,18 +19,22 @@ typedef struct timespec host_ns_time_t;
 
 static inline host_us_time_t ns_to_host_us_time(uint64_t time_ns) {
 	host_us_time_t ret;
-	ret.tv_sec = time_ns
-		/NS_PER_S;
-	ret.tv_usec = (time_ns - ((uint64_t)ret.tv_sec)*NS_PER_S)
-		/(NS_PER_S/US_PER_S);
+	ret.tv_sec = LM_SAFEDOWNCAST(typeof(ret.tv_sec),
+		time_ns
+		/NS_PER_S);
+	ret.tv_usec = LM_SAFEDOWNCAST(typeof(ret.tv_usec),
+		(time_ns - ((uint64_t)ret.tv_sec)*NS_PER_S)
+		/(NS_PER_S/US_PER_S));
 	return ret;
 }
 
 static inline host_ns_time_t ns_to_host_ns_time(uint64_t time_ns) {
 	host_ns_time_t ret;
-	ret.tv_sec = time_ns
-		/NS_PER_S;
-	ret.tv_nsec = time_ns - ((uint64_t)ret.tv_sec)*NS_PER_S;
+	ret.tv_sec = LM_SAFEDOWNCAST(typeof(ret.tv_sec),
+		time_ns
+		/NS_PER_S);
+	ret.tv_nsec = LM_SAFEDOWNCAST(typeof(ret.tv_nsec),
+		time_ns - ((uint64_t)ret.tv_sec)*NS_PER_S);
 	return ret;
 }
 
diff --git a/libhw_cr/rp2040_dma.h b/libhw_cr/rp2040_dma.h
index c7f5a8f..a6f4ae3 100644
--- a/libhw_cr/rp2040_dma.h
+++ b/libhw_cr/rp2040_dma.h
@@ -17,6 +17,7 @@
 #include <hardware/regs/dreq.h>   /* for DREQ_* for use with DMA_CTRL_TREQ_SEL() */
 #include <hardware/structs/dma.h> /* for dma_hw, dma_channel_hw_t, DMA_NUM_CHANNELS */
 
+#include <libmisc/assert.h>
 #include <libmisc/macro.h> /* for LM_FLOORLOG2() */
 
 /* Borrowed from <hardware/dma.h> *********************************************/
@@ -52,20 +53,20 @@ typedef void (*dmairq_handler_t)(void *arg, enum dmairq irq, uint channel);
  */
 void dmairq_set_and_enable_exclusive_handler(enum dmairq irq, uint channel, dmairq_handler_t fn, void *arg);
 
-#define DMA_CTRL_ENABLE          (1<<0)
-#define DMA_CTRL_HI_PRIO         (1<<1)
-#define DMA_CTRL_DATA_SIZE(sz)   ((sz)<<2)
-#define DMA_CTRL_INCR_READ       (1<<4)
-#define DMA_CTRL_INCR_WRITE      (1<<5)
-#define _DMA_CTRL_RING_BITS(b)   ((b)<<6)
-#define _DMA_CTRL_RING_RD        (0)
-#define _DMA_CTRL_RING_WR        (1<<10)
-#define DMA_CTRL_RING(rdwr, bits) (_DMA_CTRL_RING_##rdwr | _DMA_CTRL_RING_BITS(bits))
-#define DMA_CTRL_CHAIN_TO(ch)    ((ch)<<11)
-#define DMA_CTRL_TREQ_SEL(dreq)  ((dreq)<<15)
-#define DMA_CTRL_IRQ_QUIET       (1<<21)
-#define DMA_CTRL_BSWAP           (1<<22)
-#define DMA_CTRL_SNIFF_EN        (1<<23)
+#define DMA_CTRL_ENABLE              (((uint32_t)1)<<0)
+#define DMA_CTRL_HI_PRIO             (((uint32_t)1)<<1)
+#define DMA_CTRL_DATA_SIZE(sz)       (((uint32_t)(sz))<<(2 + static_assert_as_expr((sz) <= 0b11)))
+#define DMA_CTRL_INCR_READ           (((uint32_t)1)<<4)
+#define DMA_CTRL_INCR_WRITE          (((uint32_t)1)<<5)
+#define _DMA_CTRL_RING_BITS(bitcnt)  (((uint32_t)(bitcnt))<<(6 + static_assert_as_expr((bitcnt) <= 0b1111)))
+#define _DMA_CTRL_RING_RD            ((uint32_t)0)
+#define _DMA_CTRL_RING_WR            (((uint32_t)1)<<10)
+#define DMA_CTRL_RING(rdwr, bitcnt)  (_DMA_CTRL_RING_##rdwr | _DMA_CTRL_RING_BITS(bitcnt))
+#define DMA_CTRL_CHAIN_TO(ch)        (((uint32_t)(ch))<<11)
+#define DMA_CTRL_TREQ_SEL(dreq)      (((uint32_t)(dreq))<<(15 + static_assert_as_expr((dreq) <= 0b111111)))
+#define DMA_CTRL_IRQ_QUIET           (((uint32_t)1)<<21)
+#define DMA_CTRL_BSWAP               (((uint32_t)1)<<22)
+#define DMA_CTRL_SNIFF_EN            (((uint32_t)1)<<23)
 
 /*                    |          elem         |     val    |  name */
 #define READ_ADDR   /*|*/volatile const void/*|*/    *   /*|*/read_addr
diff --git a/libhw_cr/rp2040_gpioirq.c b/libhw_cr/rp2040_gpioirq.c
index 1ae74f9..8d406c6 100644
--- a/libhw_cr/rp2040_gpioirq.c
+++ b/libhw_cr/rp2040_gpioirq.c
@@ -17,8 +17,6 @@ struct gpioirq_handler_entry {
 };
 struct gpioirq_handler_entry gpioirq_handlers[NUM_BANK0_GPIOS][4] = {0};
 
-int gpioirq_core = -1;
-
 static void gpioirq_handler(void) {
 	uint core = get_core_num();
 	io_bank0_irq_ctrl_hw_t *irq_ctrl_base;
@@ -44,6 +42,8 @@ static void gpioirq_handler(void) {
 }
 
 void gpioirq_set_and_enable_exclusive_handler(uint gpio, enum gpio_irq_level event, gpioirq_handler_t fn, void *arg) {
+	static uint gpioirq_core = ~0U;
+
 	assert(gpio < NUM_BANK0_GPIOS);
 	assert(event == GPIO_IRQ_LEVEL_LOW ||
 	       event == GPIO_IRQ_LEVEL_HIGH ||
@@ -55,7 +55,7 @@ void gpioirq_set_and_enable_exclusive_handler(uint gpio, enum gpio_irq_level eve
 	assert(gpioirq_handlers[gpio][event_idx].fn == NULL);
 
 	uint core = get_core_num();
-	assert(gpioirq_core == -1 || gpioirq_core == (int)core);
+	assert(gpioirq_core == ~0U || gpioirq_core == core);
 
 	io_bank0_irq_ctrl_hw_t *irq_ctrl_base;
 	switch (core) {
@@ -67,7 +67,7 @@ void gpioirq_set_and_enable_exclusive_handler(uint gpio, enum gpio_irq_level eve
 	gpioirq_handlers[gpio][event_idx].fn = fn;
 	gpioirq_handlers[gpio][event_idx].arg = arg;
 	hw_set_bits(&irq_ctrl_base->inte[gpio/8], 1u<<((4*(gpio%8))+event_idx));
-	if (gpioirq_core == -1) {
+	if (gpioirq_core == ~0U) {
 		irq_set_exclusive_handler(IO_IRQ_BANK0, gpioirq_handler);
 		irq_set_enabled(IO_IRQ_BANK0, true);
 		gpioirq_core = core;
diff --git a/libhw_cr/rp2040_hwtimer.c b/libhw_cr/rp2040_hwtimer.c
index 8227abb..7813dfa 100644
--- a/libhw_cr/rp2040_hwtimer.c
+++ b/libhw_cr/rp2040_hwtimer.c
@@ -109,9 +109,9 @@ static bool rp2040_hwtimer_add_trigger(struct rp2040_hwtimer     *alarmclock,
 	*dst = trigger;
 	if (!alarmclock->initialized) {
 		hw_set_bits(&timer_hw->inte, 1 << alarmclock->alarm_num);
-		irq_set_exclusive_handler(TIMER_ALARM_IRQ_NUM(timer_hw, alarmclock->alarm_num),
-		                          rp2040_hwtimer_intrhandler);
-		irq_set_enabled(TIMER_ALARM_IRQ_NUM(timer_hw, alarmclock->alarm_num), true);
+		uint irq_num = (uint)TIMER_ALARM_IRQ_NUM(timer_hw, alarmclock->alarm_num);
+		irq_set_exclusive_handler(irq_num, rp2040_hwtimer_intrhandler);
+		irq_set_enabled(irq_num, true);
 		alarmclock->initialized = true;
 	}
 	if (alarmclock->queue == trigger) {
diff --git a/libhw_cr/w5500.c b/libhw_cr/w5500.c
index e676364..8ec3f4d 100644
--- a/libhw_cr/w5500.c
+++ b/libhw_cr/w5500.c
@@ -68,6 +68,10 @@
  */
 
 #include <inttypes.h> /* for PRIu{n} */
+#include <limits.h>   /* for SSIZE_MAX, not set by newlib */
+#ifndef SSIZE_MAX
+#define SSIZE_MAX (SIZE_MAX >> 1)
+#endif
 
 /* TODO: Write a <libhw/generic/gpio.h> to avoid w5500.c being
  * pico-sdk-specific.  */
@@ -318,14 +322,15 @@ void _w5500_init(struct w5500 *chip,
 #if CONFIG_W5500_VALIDATE_SPI
 	/* Validate that SPI works correctly.  */
 	bool spi_ok = true;
-	for (uint16_t a = 0; a < 0x100; a++) {
+	uint8_t a = 0;
+	do {
 		w5500ll_write_sock_reg(chip->spidev, 0, mode, a);
 		uint8_t b = w5500ll_read_sock_reg(chip->spidev, 0, mode);
 		if (b != a) {
 			log_errorf("SPI to W5500 does not appear to be functional: wrote:0x%02"PRIx16" != read:0x%02"PRIx8, a, b);
 			spi_ok = false;
 		}
-	}
+	} while (a++ != 0xff);
 	if (!spi_ok)
 		__lm_abort();
 	w5500ll_write_sock_reg(chip->spidev, 0, mode, 0);
@@ -612,6 +617,7 @@ static ssize_t w5500_tcp_writev(struct _w5500_socket *socket, const struct iovec
 	ASSERT_SELF(stream_conn, TCP);
 	if (count == 0)
 		return 0;
+	assert(count <= SSIZE_MAX);
 
 	/* What we really want is to pause until we receive an ACK for
 	 * some data we just queued, so that we can line up some new
@@ -657,7 +663,7 @@ static ssize_t w5500_tcp_writev(struct _w5500_socket *socket, const struct iovec
 
 		/* Queue data to be sent.  */
 		if ((size_t)freesize > count-done)
-			freesize = count-done;
+			freesize = (uint16_t)(count-done);
 		uint16_t ptr = uint16be_unmarshal(w5500ll_read_sock_reg(chip->spidev, socknum, tx_write_pointer));
 		w5500ll_writev(chip->spidev, ptr, CTL_BLOCK_SOCK(socknum, TX), iov, iovcnt, done, freesize);
 		w5500ll_write_sock_reg(chip->spidev, socknum, tx_write_pointer, uint16be_marshal(ptr+freesize));
@@ -681,7 +687,8 @@ static ssize_t w5500_tcp_writev(struct _w5500_socket *socket, const struct iovec
 		}
 	}
 	log_debugf(" => send finished");
-	return done;
+	assert(done <= count); /* because we validated that count <= SSIZE_MAX */
+	return (ssize_t)done;
 }
 
 static void w5500_tcp_set_read_deadline(struct _w5500_socket *socket, uint64_t ns) {
@@ -759,7 +766,7 @@ static ssize_t w5500_tcp_readv(struct _w5500_socket *socket, const struct iovec
 	uint16_t ptr = uint16be_unmarshal(w5500ll_read_sock_reg(chip->spidev, socknum, rx_read_pointer));
 	/* Read the data.  */
 	if ((size_t)avail > count)
-		avail = count;
+		avail = (uint16_t)count;
 	w5500ll_readv(chip->spidev, ptr, CTL_BLOCK_SOCK(socknum, RX), iov, iovcnt, avail);
 	/* Tell the chip that we read the data.  */
 	w5500ll_write_sock_reg(chip->spidev, socknum, rx_read_pointer, uint16be_marshal(ptr+avail));
@@ -807,18 +814,19 @@ static int w5500_tcp_close_write(struct _w5500_socket *socket) { return w5500_tc
 
 /* udp_conn methods ***********************************************************/
 
-static ssize_t w5500_udp_sendto(struct _w5500_socket *socket, void *buf, size_t count,
+static ssize_t w5500_udp_sendto(struct _w5500_socket *socket, void *buf, size_t _count,
                                 struct net_ip4_addr node, uint16_t port) {
 	log_debugf("udp_conn.sendto()");
 	ASSERT_SELF(packet_conn, UDP);
 	assert(buf);
-	assert(count);
+	assert(_count);
 
 	uint16_t bufsize = ((uint16_t)w5500ll_read_sock_reg(chip->spidev, socknum, tx_buf_size))*1024;
-	if (count > bufsize) {
+	if (_count > (size_t)bufsize) {
 		log_debugf(" => msg too large");
 		return -NET_EMSGSIZE;
 	}
+	uint16_t count = (uint16_t)_count;
 
 	for (;;) {
 		cr_mutex_lock(&chip->mu);
@@ -944,7 +952,7 @@ static ssize_t w5500_udp_recvfrom(struct _w5500_socket *socket, void *buf, size_
 				.iov_len = len,
 			}), 1, 0);
 	/* Tell the chip that we read the data.  */
-	w5500ll_write_sock_reg(chip->spidev, socknum, rx_read_pointer, uint16be_marshal(ptr+8+len));
+	w5500ll_write_sock_reg(chip->spidev, socknum, rx_read_pointer, uint16be_marshal((uint16_t)(ptr+len+8)));
 	w5500_socket_cmd(socket, CMD_RECV);
 	/* Return.  */
 	LO_CALL(bootclock, del_trigger, &trigger);