Page MenuHomeFreeBSD
Files F151213451

D49936.id155192.diff
No OneTemporary
Actions

D49936.id155192.diff
View Options

diff --git a/lib/libc/net/linkaddr.3 b/lib/libc/net/linkaddr.3
--- a/lib/libc/net/linkaddr.3
+++ b/lib/libc/net/linkaddr.3
@@ -28,7 +28,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd May 7, 2025
+.Dd May 9, 2025
.Dt LINK_ADDR 3
.Os
.Sh NAME
@@ -42,7 +42,7 @@
.In sys/types.h
.In sys/socket.h
.In net/if_dl.h
-.Ft void
+.Ft int
.Fn link_addr "const char *addr" "struct sockaddr_dl *sdl"
.Ft char *
.Fn link_ntoa "const struct sockaddr_dl *sdl"
@@ -51,9 +51,19 @@
.Sh DESCRIPTION
The routine
.Fn link_addr
-interprets character strings representing
-link-level addresses, returning binary information suitable
-for use in system calls.
+parses a character string
+.Fa addr
+representing a link-level address,
+and stores the resulting address in the structure pointed to by
+.Fa sdl .
+A link-level address consists of an optional interface name, followed by
+a colon (which is required in all cases), followed by an address
+consisting of either a string of hexadecimal digits, or a series of
+hexadecimal octets separated by one of the characters
+.Sq "." ,
+.Sq ":" ,
+or
+.Sq - .
.Pp
The routine
.Fn link_ntoa
@@ -133,10 +143,11 @@
.Pp
The
.Fn link_addr
-function
-has no return value.
-(See
-.Sx BUGS . )
+function returns 0 on success.
+If the address did not appear to be a valid link-level address, -1 is
+returned and
+.Va errno
+is set to indicate the error.
.Sh SEE ALSO
.Xr getnameinfo 3
.Sh HISTORY
@@ -154,11 +165,6 @@
The returned values for link_ntoa
reside in a static memory area.
.Pp
-The function
-.Fn link_addr
-should diagnose improperly formed input, and there should be an unambiguous
-way to recognize this.
-.Pp
If the
.Va sdl_len
field of the link socket address
diff --git a/lib/libc/net/linkaddr.c b/lib/libc/net/linkaddr.c
--- a/lib/libc/net/linkaddr.c
+++ b/lib/libc/net/linkaddr.c
@@ -36,87 +36,152 @@
#include <net/if_dl.h>
#include <assert.h>
+#include <errno.h>
#include <stdint.h>
#include <string.h>
-/* States*/
-#define NAMING 0
-#define GOTONE 1
-#define GOTTWO 2
-#define RESET 3
-/* Inputs */
-#define DIGIT (4*0)
-#define END (4*1)
-#define DELIM (4*2)
-#define LETTER (4*3)
-
-void
+int
link_addr(const char *addr, struct sockaddr_dl *sdl)
{
char *cp = sdl->sdl_data;
char *cplim = sdl->sdl_len + (char *)sdl;
- int byte = 0, state = NAMING, new;
+ const char *nptr;
+ char delim = 0;
+ size_t newsize;
+ int error = 0;
+ /* Initialise the sdl to zero, except for sdl_len. */
bzero((char *)&sdl->sdl_family, sdl->sdl_len - 1);
sdl->sdl_family = AF_LINK;
- do {
- state &= ~LETTER;
- if ((*addr >= '0') && (*addr <= '9')) {
- new = *addr - '0';
- } else if ((*addr >= 'a') && (*addr <= 'f')) {
- new = *addr - 'a' + 10;
- } else if ((*addr >= 'A') && (*addr <= 'F')) {
- new = *addr - 'A' + 10;
- } else if (*addr == 0) {
- state |= END;
- } else if (state == NAMING &&
- (((*addr >= 'A') && (*addr <= 'Z')) ||
- ((*addr >= 'a') && (*addr <= 'z'))))
- state |= LETTER;
- else
- state |= DELIM;
- addr++;
- switch (state /* | INPUT */) {
- case NAMING | DIGIT:
- case NAMING | LETTER:
- *cp++ = addr[-1];
- continue;
- case NAMING | DELIM:
- state = RESET;
- sdl->sdl_nlen = cp - sdl->sdl_data;
- continue;
- case GOTTWO | DIGIT:
- *cp++ = byte;
- /* FALLTHROUGH */
- case RESET | DIGIT:
- state = GOTONE;
- byte = new;
- continue;
- case GOTONE | DIGIT:
- state = GOTTWO;
- byte = new + (byte << 4);
- continue;
- default: /* | DELIM */
- state = RESET;
- *cp++ = byte;
- byte = 0;
- continue;
- case GOTONE | END:
- case GOTTWO | END:
- *cp++ = byte;
- /* FALLTHROUGH */
- case RESET | END:
+
+ /*
+ * Everything up to the first ':' is the interface name. Usually the
+ * ':' should always be present even if there's no interface name, but
+ * since this interface was poorly specified in the past, accept a
+ * missing colon as meaning no interface name.
+ */
+ if ((nptr = strchr(addr, ':')) != NULL) {
+ size_t namelen = nptr - addr;
+
+ /* Ensure the sdl is large enough to store the name. */
+ if (namelen > (cplim - cp)) {
+ errno = ENOSPC;
+ return (-1);
+ }
+
+ memcpy(cp, addr, namelen);
+ cp += namelen;
+ sdl->sdl_nlen = namelen;
+ /* Skip the interface name and the colon. */
+ addr += namelen + 1;
+ }
+
+ /*
+ * The remainder of the string should be hex digits representing the
+ * address, with optional delimiters. Each two hex digits form one
+ * octet, but octet output can be forced using a delimiter, so we accept
+ * a long string of hex digits, or a mix of delimited and undelimited
+ * digits like "1122.3344.5566", or delimited one- or two-digit octets
+ * like "1.22.3".
+ *
+ * If anything fails at this point, exit the loop so we set sdl_alen and
+ * sdl_len based on whatever we did manage to parse. This preserves
+ * compatibility with the 4.3BSD version of link_addr, which had no way
+ * to indicate an error and would just return.
+ */
+#define DIGIT(c) \
+ (((c) >= '0' && (c) <= '9') ? (c - '0') \
+ : ((c) >= 'a' && (c) <= 'f') ? (c - 'a' + 10) \
+ : ((c) >= 'A' && (c) <= 'F') ? (c - 'A' + 10) \
+ : (-1))
+#define ISDELIM(c) (((c) != 0) && (strchr(".:-", (c)) != NULL) \
+ && (delim == 0 || delim == (c)))
+
+ for (;;) {
+ int digit, digit2;
+
+ /*
+ * Treat any leading delimiters as empty bytes. This supports
+ * the (somewhat obsolete) form of Ethernet addresses with empty
+ * octets, e.g. "1::3:4:5:6".
+ */
+ while (ISDELIM(*addr) && (cp < cplim)) {
+ delim = *addr++;
+ *cp++ = 0;
+ }
+
+ /* Did we reach the end of the string? */
+ if (*addr == '\0')
+ break;
+
+ /*
+ * If not, the next character must be a digit, so make sure we
+ * have room for at least one more octet.
+ */
+
+ if (cp >= cplim) {
+ error = ENOSPC;
break;
}
- break;
- } while (cp < cplim);
+
+ if ((digit = DIGIT(*addr)) == -1) {
+ error = EINVAL;
+ break;
+ }
+
+ /* See what the next character is */
+ ++addr;
+
+ if (*addr == '\0') {
+ /* If the digit is followed by EOS, we're done. */
+ *cp++ = digit;
+ break;
+ } else if (ISDELIM(*addr)) {
+ /*
+ * If the digit is followed by a delimiter, write the
+ * nibble.
+ */
+ delim = *addr++;
+ *cp++ = digit;
+ } else if ((digit2 = DIGIT(*addr)) != -1) {
+ /*
+ * If the digit is followed by another digit, write the
+ * octet.
+ */
+ *cp++ = (digit << 4) | digit2;
+ ++addr;
+
+ /* Skip any delimiter after this octet */
+ if (ISDELIM(*addr))
+ delim = *addr++;
+ } else {
+ /* Otherwise, the input was invalid. */
+ error = EINVAL;
+ break;
+ }
+ }
+#undef DIGIT
+#undef ISDELIM
+
+ /* How many bytes did we write to the address? */
sdl->sdl_alen = cp - LLADDR(sdl);
- new = cp - (char *)sdl;
- if (new > sizeof(*sdl))
- sdl->sdl_len = new;
- return;
+
+ /*
+ * The user might have given us an sdl which is larger than sizeof(sdl);
+ * in that case, record the actual size of the new sdl.
+ */
+ newsize = cp - (char *)sdl;
+ if (newsize > sizeof(*sdl))
+ sdl->sdl_len = (u_char)newsize;
+
+ if (error == 0)
+ return (0);
+
+ errno = error;
+ return (-1);
}
+
char *
link_ntoa(const struct sockaddr_dl *sdl)
{
diff --git a/lib/libc/tests/net/link_addr_test.cc b/lib/libc/tests/net/link_addr_test.cc
--- a/lib/libc/tests/net/link_addr_test.cc
+++ b/lib/libc/tests/net/link_addr_test.cc
@@ -71,10 +71,12 @@
make_linkaddr(const std::string &addr)
{
auto sdl = sockaddr_dl{};
+ int ret;
sdl.sdl_len = sizeof(sdl);
- ::link_addr(addr.c_str(), &sdl);
+ ret = ::link_addr(addr.c_str(), &sdl);
+ ATF_REQUIRE_EQ(0, ret);
ATF_REQUIRE_EQ(AF_LINK, static_cast<int>(sdl.sdl_family));
ATF_REQUIRE_EQ(true, sdl.sdl_len > 0);
ATF_REQUIRE_EQ(true, sdl.sdl_nlen >= 0);
@@ -176,6 +178,10 @@
{"aa:bb:cc:dd:ee:ff"s, "aa.bb.cc.dd.ee.ff",
ether_addr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}},
+
+ // Address with a blank octet; this is an old form of Ethernet address.
+ {"00:11::33:44:55"s, "0.11.0.33.44.55",
+ ether_addr{0x00, 0x11, 0x00, 0x33, 0x44, 0x55}},
};
/*
@@ -219,6 +225,43 @@
}
+/*
+ * Test with some invalid addresses.
+ */
+ATF_TEST_CASE_WITHOUT_HEAD(invalid)
+ATF_TEST_CASE_BODY(invalid)
+{
+ auto const invalid_addresses = std::vector{
+ // Invalid separator
+ ":1/2/3"s,
+ "ix0:1/2/3"s,
+
+ // Multiple different separators
+ ":1.2-3"s,
+ "ix0:1.2-3"s,
+
+ // An IP address
+ ":10.1.2.200/28"s,
+ "ix0:10.1.2.200/28"s,
+
+ // Valid address followed by garbage
+ ":1.2.3xxx"s,
+ ":1.2.3.xxx"s,
+ "ix0:1.2.3xxx"s,
+ "ix0:1.2.3.xxx"s,
+ };
+
+ for (auto const &addr : invalid_addresses) {
+ int ret;
+
+ auto sdl = sockaddr_dl{};
+ sdl.sdl_len = sizeof(sdl);
+
+ ret = link_addr(addr.c_str(), &sdl);
+ ATF_REQUIRE_EQ(-1, ret);
+ }
+}
+
/*
* Test some non-Ethernet addresses.
*/
@@ -245,6 +288,111 @@
lladdr(sdl)));
}
+/*
+ * Test link_addr behaviour with undersized buffers.
+ */
+ATF_TEST_CASE_WITHOUT_HEAD(smallbuf)
+ATF_TEST_CASE_BODY(smallbuf)
+{
+ static constexpr auto garbage = std::byte{0xcc};
+ auto buf = std::vector<std::byte>();
+ sockaddr_dl *sdl;
+ int ret;
+
+ /*
+ * Make an sdl with an sdl_data member of the appropriate size, and
+ * place it in buf. Ensure it's followed by a trailing byte of garbage
+ * so we can test that link_addr doesn't write past the end.
+ */
+ auto mksdl = [&buf](std::size_t datalen) -> sockaddr_dl * {
+ auto actual_size = datalen + offsetof(sockaddr_dl, sdl_data);
+
+ buf.resize(actual_size + 1);
+ std::ranges::fill(buf, garbage);
+
+ auto *sdl = new(reinterpret_cast<sockaddr_dl *>(&buf[0]))
+ sockaddr_dl;
+ sdl->sdl_len = actual_size;
+ return (sdl);
+ };
+
+ /* An sdl large enough to store the interface name */
+ sdl = mksdl(3);
+ ret = link_addr("ix0:1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(-1, ret);
+ ATF_REQUIRE_EQ(ENOSPC, errno);
+ ATF_REQUIRE_EQ(3, sdl->sdl_nlen);
+ ATF_REQUIRE_EQ("ix0", ifname(*sdl));
+ ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
+
+ /*
+ * For these tests, test both with and without an interface name, since
+ * that will overflow the buffer in different places.
+ */
+
+ /* An empty sdl. Nothing may grow on this cursed ground. */
+
+ sdl = mksdl(0);
+ ret = link_addr("ix0:1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(-1, ret);
+ ATF_REQUIRE_EQ(ENOSPC, errno);
+ ATF_REQUIRE_EQ(0, sdl->sdl_nlen);
+ ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
+
+ sdl = mksdl(0);
+ ret = link_addr(":1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(-1, ret);
+ ATF_REQUIRE_EQ(ENOSPC, errno);
+ ATF_REQUIRE_EQ(0, sdl->sdl_nlen);
+ ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
+
+ /*
+ * An sdl large enough to store the interface name and two octets of the
+ * address.
+ */
+
+ sdl = mksdl(5);
+ ret = link_addr("ix0:1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(-1, ret);
+ ATF_REQUIRE_EQ(ENOSPC, errno);
+ ATF_REQUIRE_EQ("ix0", ifname(*sdl));
+ ATF_REQUIRE(std::ranges::equal(
+ std::vector{ 0x01, 0x02 }, lladdr(*sdl)));
+
+ sdl = mksdl(2);
+ ret = link_addr(":1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(-1, ret);
+ ATF_REQUIRE_EQ(ENOSPC, errno);
+ ATF_REQUIRE_EQ("", ifname(*sdl));
+ ATF_REQUIRE(std::ranges::equal(
+ std::vector{ 0x01, 0x02 }, lladdr(*sdl)));
+
+ /*
+ * An sdl large enough to store the entire address.
+ */
+
+ sdl = mksdl(6);
+ ret = link_addr("ix0:1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(0, ret);
+ ATF_REQUIRE_EQ("ix0", ifname(*sdl));
+ ATF_REQUIRE(std::ranges::equal(
+ std::vector{ 0x01, 0x02, 0x03 }, lladdr(*sdl)));
+
+ sdl = mksdl(3);
+ ret = link_addr(":1.2.3", sdl);
+ ATF_REQUIRE(*rbegin(buf) == garbage);
+ ATF_REQUIRE_EQ(0, ret);
+ ATF_REQUIRE_EQ("", ifname(*sdl));
+ ATF_REQUIRE(std::ranges::equal(
+ std::vector{ 0x01, 0x02, 0x03 }, lladdr(*sdl)));
+}
+
/*
* Test an extremely long address which would overflow link_ntoa's internal
* buffer. It should handle this by truncating the output.
@@ -376,6 +524,8 @@
{
ATF_ADD_TEST_CASE(tcs, basic);
ATF_ADD_TEST_CASE(tcs, ifname);
+ ATF_ADD_TEST_CASE(tcs, smallbuf);
+ ATF_ADD_TEST_CASE(tcs, invalid);
ATF_ADD_TEST_CASE(tcs, nonether);
ATF_ADD_TEST_CASE(tcs, overlong);
ATF_ADD_TEST_CASE(tcs, link_ntoa_r);
diff --git a/sbin/ifconfig/af_link.c b/sbin/ifconfig/af_link.c
--- a/sbin/ifconfig/af_link.c
+++ b/sbin/ifconfig/af_link.c
@@ -210,7 +210,8 @@
temp[0] = ':';
strcpy(temp + 1, addr);
sdl.sdl_len = sizeof(sdl);
- link_addr(temp, &sdl);
+ if (link_addr(temp, &sdl) == -1)
+ errx(1, "malformed link-level address");
free(temp);
}
if (sdl.sdl_alen > sizeof(sa->sa_data))
diff --git a/sys/net/if_dl.h b/sys/net/if_dl.h
--- a/sys/net/if_dl.h
+++ b/sys/net/if_dl.h
@@ -82,7 +82,7 @@
#include <sys/cdefs.h>
__BEGIN_DECLS
-void link_addr(const char *, struct sockaddr_dl *);
+int link_addr(const char *, struct sockaddr_dl *);
char *link_ntoa(const struct sockaddr_dl *);
int link_ntoa_r(const struct sockaddr_dl *, char *, size_t *);
__END_DECLS
diff --git a/sys/sys/param.h b/sys/sys/param.h
--- a/sys/sys/param.h
+++ b/sys/sys/param.h
@@ -73,7 +73,7 @@
* cannot include sys/param.h and should only be updated here.
*/
#undef __FreeBSD_version
-#define __FreeBSD_version 1500042
+#define __FreeBSD_version 1500043
/*
* __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,

File Metadata

Mime Type
text/plain
Expires
Tue, Apr 7, 9:31 PM (12 h, 56 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
31050379
Default Alt Text
D49936.id155192.diff (13 KB)

Event Timeline