Skip to content

Commit b5f0de6

Browse files
keeskuba-moo
authored andcommitted
net: dev: Convert sa_data to flexible array in struct sockaddr
One of the worst offenders of "fake flexible arrays" is struct sockaddr, as it is the classic example of why GCC and Clang have been traditionally forced to treat all trailing arrays as fake flexible arrays: in the distant misty past, sa_data became too small, and code started just treating it as a flexible array, even though it was fixed-size. The special case by the compiler is specifically that sizeof(sa->sa_data) and FORTIFY_SOURCE (which uses __builtin_object_size(sa->sa_data, 1)) do not agree (14 and -1 respectively), which makes FORTIFY_SOURCE treat it as a flexible array. However, the coming -fstrict-flex-arrays compiler flag will remove these special cases so that FORTIFY_SOURCE can gain coverage over all the trailing arrays in the kernel that are _not_ supposed to be treated as a flexible array. To deal with this change, convert sa_data to a true flexible array. To keep the structure size the same, move sa_data into a union with a newly introduced sa_data_min with the original size. The result is that FORTIFY_SOURCE can continue to have no idea how large sa_data may actually be, but anything using sizeof(sa->sa_data) must switch to sizeof(sa->sa_data_min). Cc: Jens Axboe <[email protected]> Cc: Pavel Begunkov <[email protected]> Cc: David Ahern <[email protected]> Cc: Dylan Yudaken <[email protected]> Cc: Yajun Deng <[email protected]> Cc: Petr Machata <[email protected]> Cc: Hangbin Liu <[email protected]> Cc: Leon Romanovsky <[email protected]> Cc: syzbot <[email protected]> Cc: Willem de Bruijn <[email protected]> Cc: Pablo Neira Ayuso <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent d6dd508 commit b5f0de6

File tree

4 files changed

+11
-8
lines changed

4 files changed

+11
-8
lines changed

include/linux/socket.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ typedef __kernel_sa_family_t sa_family_t;
3333

3434
struct sockaddr {
3535
sa_family_t sa_family; /* address family, AF_xxx */
36-
char sa_data[14]; /* 14 bytes of protocol address */
36+
union {
37+
char sa_data_min[14]; /* Minimum 14 bytes of protocol address */
38+
DECLARE_FLEX_ARRAY(char, sa_data);
39+
};
3740
};
3841

3942
struct linger {

net/core/dev.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8822,7 +8822,7 @@ EXPORT_SYMBOL(dev_set_mac_address_user);
88228822

88238823
int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name)
88248824
{
8825-
size_t size = sizeof(sa->sa_data);
8825+
size_t size = sizeof(sa->sa_data_min);
88268826
struct net_device *dev;
88278827
int ret = 0;
88288828

net/core/dev_ioctl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
342342
if (ifr->ifr_hwaddr.sa_family != dev->type)
343343
return -EINVAL;
344344
memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
345-
min(sizeof(ifr->ifr_hwaddr.sa_data),
345+
min(sizeof(ifr->ifr_hwaddr.sa_data_min),
346346
(size_t)dev->addr_len));
347347
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
348348
return 0;

net/packet/af_packet.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3277,7 +3277,7 @@ static int packet_bind_spkt(struct socket *sock, struct sockaddr *uaddr,
32773277
int addr_len)
32783278
{
32793279
struct sock *sk = sock->sk;
3280-
char name[sizeof(uaddr->sa_data) + 1];
3280+
char name[sizeof(uaddr->sa_data_min) + 1];
32813281

32823282
/*
32833283
* Check legality
@@ -3288,8 +3288,8 @@ static int packet_bind_spkt(struct socket *sock, struct sockaddr *uaddr,
32883288
/* uaddr->sa_data comes from the userspace, it's not guaranteed to be
32893289
* zero-terminated.
32903290
*/
3291-
memcpy(name, uaddr->sa_data, sizeof(uaddr->sa_data));
3292-
name[sizeof(uaddr->sa_data)] = 0;
3291+
memcpy(name, uaddr->sa_data, sizeof(uaddr->sa_data_min));
3292+
name[sizeof(uaddr->sa_data_min)] = 0;
32933293

32943294
return packet_do_bind(sk, name, 0, pkt_sk(sk)->num);
32953295
}
@@ -3561,11 +3561,11 @@ static int packet_getname_spkt(struct socket *sock, struct sockaddr *uaddr,
35613561
return -EOPNOTSUPP;
35623562

35633563
uaddr->sa_family = AF_PACKET;
3564-
memset(uaddr->sa_data, 0, sizeof(uaddr->sa_data));
3564+
memset(uaddr->sa_data, 0, sizeof(uaddr->sa_data_min));
35653565
rcu_read_lock();
35663566
dev = dev_get_by_index_rcu(sock_net(sk), READ_ONCE(pkt_sk(sk)->ifindex));
35673567
if (dev)
3568-
strscpy(uaddr->sa_data, dev->name, sizeof(uaddr->sa_data));
3568+
strscpy(uaddr->sa_data, dev->name, sizeof(uaddr->sa_data_min));
35693569
rcu_read_unlock();
35703570

35713571
return sizeof(*uaddr);

0 commit comments

Comments
 (0)