Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 49 additions & 7 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,19 @@ EOF
esac

if [ "$SANITIZEADDRESS" = yes ]; then
printf "Testing compiler supports address sanitisation ..."
printf "Testing compiler supports address sanitisation ... "
cat <<EOF >_test.c
int main(void) {
return 0;
}
EOF
if $CC -fsanitize=address _test.c -o _test 2>&3; then
if $CC -fsanitize=address,undefined _test.c -o _test 2>&3; then
echo "yes"
echo "SANITIZEADDRESS= yes" >>$CONFIG_MK
echo "CPPFLAGS+= -DASAN" >>$CONFIG_MK
echo "CFLAGS+= -fsanitize=address" >>$CONFIG_MK
echo "CFLAGS+= -fsanitize=address,undefined" >>$CONFIG_MK
echo "CFLAGS+= -fno-omit-frame-pointer" >>$CONFIG_MK
echo "LDFLAGS+= -fsanitize=address" >>$CONFIG_MK
echo "LDFLAGS+= -fsanitize=address,undefined" >>$CONFIG_MK
else
echo "no"
fi
Expand Down Expand Up @@ -591,6 +591,50 @@ fi
rm -rf _inet_ntoa.* _inet_ntoa
$abort && exit 1

if [ -z "$LINK_SRC" ]; then
printf "Testing for route ... "
cat << EOF >_route.c
#include <sys/socket.h>
#include <sys/types.h>
#include <net/route.h>
int main(void) {
struct rt_msghdr rtm = { .rtm_type = RTM_IFINFO };
return (int)rtm.rtm_type;
}
EOF
if $XCC _route.c -o _route 2>&3; then
LINK_SRC="route.c"
echo "yes"
else
echo "no"
fi
rm -rf _route.* _route
fi

if [ -z "$LINK_SRC" ]; then
printf "Testing for netlink ... "
cat << EOF >_netlink.c
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <sys/socket.h>
int main(void) {
return 0;
}
EOF
if $XCC _netlink.c -o _netlink 2>&3; then
LINK_SRC="netlink.c"
echo "yes"
else
echo "no"
fi
rm -rf _netlink.* _netlink
fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if [ -z "$LINK_SRC" ]; then
echo "no mechanism for detecting interface departures found" >&2
exit 1
fi
echo "LINK_SRC= $LINK_SRC" >>$CONFIG_MK

if [ -z "$SETPROCTITLE" ]; then
printf "Testing for setproctitle ... "
cat << EOF >_setproctitle.c
Expand Down Expand Up @@ -742,7 +786,5 @@ echo " LIBDIR = $LIBDIR"
echo " DATADIR = $DATADIR"
echo " RUNDIR = $RUNDIR"
echo " MANDIR = $MANDIR"
if [ "$PRIVSEP" = yes ]; then
echo " PRIVSEPUSER = $PRIVSEP_USER"
fi
echo " DHCPSD_USER = $DHCPSD_USER"
echo
9 changes: 5 additions & 4 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ PROG= dhcpsd
SRCS= dhcpsd.c common.c eloop.c if.c logerr.c
SRCS+= bpf.c dhcp.c dhcp_lease.c
SRCS+= if_none.c if_ether.c
SRCS+= service.c unpriv.c
SRCS+= service.c priv.c unpriv.c
SRCS+= plugin.c
OBJS+= ${SRCS:.c=.o}
SRCS+= ${LINK_SRC}
OBJS+= ${SRCS:.c=.o}

PVENDOR_SRCS= ${VENDOR_SRCS:vendor/%=${TOP}/vendor/%}
OBJS+= ${PVENDOR_SRCS:.c=.o}
OBJS+= ${PVENDOR_SRCS:.c=.o}

PCOMPAT_SRCS= ${COMPAT_SRCS:compat/%=${TOP}/compat/%}
OBJS+= ${PCOMPAT_SRCS:.c=.o}
OBJS+= ${PCOMPAT_SRCS:.c=.o}

CFLAGS?= -O2
SUBDIRS= plugins
Expand Down
7 changes: 3 additions & 4 deletions src/bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ bpf_open(const struct interface *ifp, int (*filter)(const struct bpf *),
#else
struct bpf_version pv = { .bv_major = 0, .bv_minor = 0 };
struct ifreq ifr = { .ifr_flags = 0 };
#if 0
#if 1
int ibuf_len = 0;
#endif
Comment on lines +400 to 402
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e

printf 'bpf_open call sites and nearby flags:\n\n'
rg -nP -C3 '\bbpf_open\s*\(' src

printf '\nCurrent allocation guard in src/bpf.c:\n\n'
sed -n '505,518p' src/bpf.c

Repository: NetworkConfiguration/dhcpsd

Length of output: 1385


🏁 Script executed:

#!/bin/bash

# 1. Check POSIX flag values in standard headers
echo "=== POSIX O_* flag definitions ===" 
grep -rn "define O_RDONLY\|define O_WRONLY\|define O_RDWR\|define O_ACCMODE" /usr/include --include="*.h" 2>/dev/null | head -20

echo ""
echo "=== All bpf_open call sites ===" 
rg -n '\bbpf_open\s*\(' src

echo ""
echo "=== Context: src/bpf.c lines 395-410 (around line 400) ===" 
sed -n '395,410p' src/bpf.c

echo ""
echo "=== Context: src/bpf.c lines 505-520 (allocation guard) ===" 
sed -n '505,520p' src/bpf.c

echo ""
echo "=== Check for bpf_read definition and usage ===" 
grep -n "^bpf_read" src/bpf.c | head -5
rg -n 'bpf_read\s*\(' src --max-count=10

Repository: NetworkConfiguration/dhcpsd

Length of output: 1372


🏁 Script executed:

#!/bin/bash

echo "=== All invocations of bpf_read() ===" 
rg -n '\bbpf_read\s*\(' src

echo ""
echo "=== bpf_read function body (lines 534-560) ===" 
sed -n '534,560p' src/bpf.c

echo ""
echo "=== Check lines 530-584 context mentioned in review ===" 
sed -n '530,545p' src/bpf.c

echo ""
echo "=== Verify POSIX flag semantics via C headers ===" 
cat > /tmp/check_flags.c << 'EOF'
`#include` <fcntl.h>
`#include` <stdio.h>
int main() {
    printf("O_RDONLY=%d\nO_WRONLY=%d\nO_RDWR=%d\nO_ACCMODE=%d\n",
           O_RDONLY, O_WRONLY, O_RDWR, O_ACCMODE);
    printf("\nO_RDONLY & (O_RDONLY | O_RDWR) = %d\n", O_RDONLY & (O_RDONLY | O_RDWR));
    printf("O_RDWR & (O_RDONLY | O_RDWR) = %d\n", O_RDWR & (O_RDONLY | O_RDWR));
    printf("(O_RDONLY & O_ACCMODE) != O_WRONLY: %d\n", (O_RDONLY & O_ACCMODE) != O_WRONLY);
    printf("(O_RDWR & O_ACCMODE) != O_WRONLY: %d\n", (O_RDWR & O_ACCMODE) != O_WRONLY);
    printf("(O_WRONLY & O_ACCMODE) != O_WRONLY: %d\n", (O_WRONLY & O_ACCMODE) != O_WRONLY);
    return 0;
}
EOF
gcc /tmp/check_flags.c -o /tmp/check_flags && /tmp/check_flags

Repository: NetworkConfiguration/dhcpsd

Length of output: 1549


🏁 Script executed:

#!/bin/bash

echo "=== Check if bpf_read is exported or used elsewhere ===" 
rg -n 'bpf_read' src --type c --type h

echo ""
echo "=== Verify the flags semantics with known values ===" 
python3 << 'EOF'
# POSIX standard values
O_RDONLY = 0
O_WRONLY = 1
O_RDWR = 2
O_ACCMODE = 3

print("POSIX flag values:")
print(f"  O_RDONLY = {O_RDONLY}")
print(f"  O_WRONLY = {O_WRONLY}")
print(f"  O_RDWR = {O_RDWR}")
print(f"  O_ACCMODE = {O_ACCMODE}")

print("\nCurrent guard: if (flags & (O_RDONLY | O_WRONLY))")
print(f"  With O_RDONLY: {O_RDONLY} & ({O_RDONLY} | {O_RDWR}) = {O_RDONLY & (O_RDONLY | O_RDWR)} (False - FAILS TO ALLOCATE)")
print(f"  With O_WRONLY: {O_WRONLY} & ({O_RDONLY} | {O_RDWR}) = {O_WRONLY & (O_RDONLY | O_RDWR)} (False - expected)")
print(f"  With O_RDWR:   {O_RDWR} & ({O_RDONLY} | {O_RDWR}) = {O_RDWR & (O_RDONLY | O_RDWR)} (True - correct)")

print("\nProposed fix: if ((flags & O_ACCMODE) != O_WRONLY)")
print(f"  With O_RDONLY: ({O_RDONLY} & {O_ACCMODE}) != {O_WRONLY} = {(O_RDONLY & O_ACCMODE) != O_WRONLY} (True - correct)")
print(f"  With O_WRONLY: ({O_WRONLY} & {O_ACCMODE}) != {O_WRONLY} = {(O_WRONLY & O_ACCMODE) != O_WRONLY} (False - correct)")
print(f"  With O_RDWR:   ({O_RDWR} & {O_ACCMODE}) != {O_WRONLY} = {(O_RDWR & O_ACCMODE) != O_WRONLY} (True - correct)")
EOF

echo ""
echo "=== Check bpf_read at line 543 for buffer access ===" 
sed -n '540,545p' src/bpf.c

Repository: NetworkConfiguration/dhcpsd

Length of output: 1023


Allocate the BPF read buffer by access mode, not O_RDONLY | O_RDWR.

Re-enabling bpf_read() makes this path depend on bpf->bpf_buffer, but the allocation check uses flags & (O_RDONLY | O_RDWR). Since O_RDONLY is 0, this condition evaluates to flags & 2, which fails for read-only opens and leaves bpf_buffer as NULL and bpf_size as 0. A read-only open to bpf_read() will then call read(bpf->bpf_fd, NULL, 0) at line 543, causing undefined behavior.

Proposed fix
-	if (flags & (O_RDONLY | O_RDWR)) {
+	if ((flags & O_ACCMODE) != O_WRONLY) {
 		/* Get the required BPF buffer length from the kernel. */
 		if (ioctl(bpf->bpf_fd, BIOCGBLEN, &ibuf_len) == -1)
 			goto eexit;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/bpf.c` around lines 400 - 402, The allocation check for bpf->bpf_buffer
must test the open access mode correctly instead of using flags & (O_RDONLY |
O_RDWR) (O_RDONLY == 0), so modify the allocation logic (in the bpf open/init
path that sets bpf->bpf_buffer and bpf_size) to inspect (flags & O_ACCMODE) and
allocate when the mode is O_RDONLY or O_RDWR (or equivalent read-capable modes);
update the condition that gates buffer allocation to use (flags & O_ACCMODE) ==
O_RDONLY || (flags & O_ACCMODE) == O_RDWR (or test for read-capable flag),
ensure bpf_size > 0 when allocating, and keep bpf_read() usage consistent so
read(bpf->bpf_fd, bpf->bpf_buffer, bpf_size) never happens with a NULL buffer or
zero size.

unsigned int imm;
Expand Down Expand Up @@ -506,7 +506,6 @@ bpf_open(const struct interface *ifp, int (*filter)(const struct bpf *),
#ifdef __linux__
UNUSED(flags);
#else
#if 0
if (flags & (O_RDONLY | O_RDWR)) {
/* Get the required BPF buffer length from the kernel. */
if (ioctl(bpf->bpf_fd, BIOCGBLEN, &ibuf_len) == -1)
Expand All @@ -517,7 +516,6 @@ bpf_open(const struct interface *ifp, int (*filter)(const struct bpf *),
if (bpf->bpf_buffer == NULL)
goto eexit;
}
#endif
#endif

return bpf;
Expand Down Expand Up @@ -568,7 +566,8 @@ bpf_read(struct bpf *bpf, void *data, size_t len)
bytes = (ssize_t)len;
else
bytes = (ssize_t)packet.bh_caplen;
memcpy(data, payload, (size_t)bytes);
if (data)
memcpy(data, payload, (size_t)bytes);
next:
bpf->bpf_pos += BPF_WORDALIGN(
packet.bh_hdrlen + packet.bh_caplen);
Expand Down
23 changes: 22 additions & 1 deletion src/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@

#include <netinet/in.h>

#ifdef AF_PACKET
#include <netpacket/packet.h>
#endif

#include <arpa/inet.h>
#include <arpa/nameser.h>
#include <ctype.h>
Expand Down Expand Up @@ -414,13 +418,30 @@ inet_ntocidr(struct in_addr *addr)
return cidr;
}

size_t
int
sa_is_link(const struct sockaddr *sa)
{
#ifdef AF_LINK
return sa->sa_family == AF_LINK ? 1 : 0;
#elif defined(AF_PACKET)
return sa->sa_family == AF_PACKET ? 1 : 0;
#else
errno = EAFNOSUPPORT;
return -1;
#endif
}

socklen_t
sa_len(const struct sockaddr *sa)
{
#ifdef BSD
return sa->sa_len;
#else
switch (sa->sa_family) {
#ifdef AF_PACKET
case AF_PACKET:
return sizeof(struct sockaddr_ll);
#endif
case AF_INET:
return sizeof(struct sockaddr_in);
case AF_INET6:
Expand Down
10 changes: 9 additions & 1 deletion src/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
#define UNUSED(a) (void)(a)
#define ARRAYCOUNT(a) (sizeof((a)) / sizeof((a)[0]))

#ifndef ALIGNBYTES
#define ALIGNBYTES (sizeof(int) - 1)
#endif
#ifndef ALIGN
#define ALIGN(p) (((unsigned int)(p) + ALIGNBYTES) & ~ALIGNBYTES)
#endif

/* Some systems don't define timespec macros */
#ifndef timespecclear
#define timespecclear(tsp) (tsp)->tv_sec = (time_t)((tsp)->tv_nsec = 0L)
Expand Down Expand Up @@ -100,7 +107,8 @@ uint8_t inet_ntocidr(struct in_addr *);

struct sockaddr;

size_t sa_len(const struct sockaddr *);
int sa_is_link(const struct sockaddr *);
socklen_t sa_len(const struct sockaddr *);
int sa_cmp(const struct sockaddr *, const struct sockaddr *);
#define ss_len(ss) sa_len((const struct sockaddr *)(ss))
int sa_pton(struct sockaddr *, const char *restrict);
Expand Down
Loading