github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-11-07 06:17:02 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

net: rmnet: annotate data-races around port->data_format

Browse Source 
port->data_format is written under RTNL from rmnet_newlink() and
rmnet_changelink(), but it is read from the data path without any
lock, both from RX (rmnet_rx_handler() context) and TX
(rmnet_egress_handler() context).

Note that rmnet_newlink() can be called for a real_dev which is
already hooked to rmnet: the rx_handler is live and traffic can
already be flowing when port->data_format is overwritten.

Add the missing READ_ONCE()/WRITE_ONCE() annotations.

While at it, sample port->data_format only once per packet and pass
the value down, so that all the decisions taken for a given packet
are based on a single consistent value.

Otherwise the two ends of a packet transformation could disagree.
On TX, rmnet_map_egress_handler() sized the headroom from one read
while rmnet_map_add_map_header() decided on MAP_NEXT_HEADER_FLAG
from another one, so a concurrent rmnet_changelink() could produce
a MAP header announcing a v5 csum header that was neither reserved
nor written. On RX, rmnet_map_validate_packet_len() and
__rmnet_map_ingress_handler() could likewise disagree on the
expected layout, and rmnet_map_send_ack() could trim a dl csum
trailer that the ingress path never accounted for.

rmnet_map_add_map_header(), rmnet_map_command(),
rmnet_map_deaggregate(), rmnet_map_send_ack(),
rmnet_map_validate_packet_len() and __rmnet_map_ingress_handler()
therefore get the value from their caller instead of re-reading it.

rmnet_changelink() now computes the new value in a local variable
and publishes it with a single store, instead of letting the data
path observe the intermediate (old_data_format & ~flags->mask)
value.

rmnet_vnd_headroom() is left alone, all its callers hold RTNL.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Sean Tranchetti <sean.tranchetti@oss.qualcomm.com>
Reviewed-by: Subash Abhinov Kasiviswanathan <subash.a.kasiviswanathan@oss.qualcomm.com>
Link: https://patch.msgid.link/20260916132231.1780091-2-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Eric Dumazet 2026-09-16 13:22:29 +00:00 committed by Jakub Kicinski
parent 4a67649006
commit c863bf6a7d
5 changed files with 42 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
View File

@ -176,7 +176,7 @@ static int rmnet_newlink(struct net_device *dev,
}
netdev_dbg(dev, "data format [0x%08X]\n", data_format);
port->data_format = data_format;
WRITE_ONCE(port->data_format, data_format);
return 0;
@ -348,14 +348,16 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
if (data[IFLA_RMNET_FLAGS]) {
struct ifla_rmnet_flags *flags;
u32 old_data_format;
u32 data_format;
old_data_format = port->data_format;
flags = nla_data(data[IFLA_RMNET_FLAGS]);
port->data_format &= ~flags->mask;
port->data_format |= flags->flags & flags->mask;
data_format = old_data_format & ~flags->mask;
data_format |= flags->flags & flags->mask;
WRITE_ONCE(port->data_format, data_format);
if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
port->data_format = old_data_format;
WRITE_ONCE(port->data_format, old_data_format);
NL_SET_ERR_MSG_MOD(extack, "Invalid MTU on real dev");
return -EINVAL;
}

32
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
View File

@ -54,7 +54,8 @@ rmnet_deliver_skb(struct sk_buff *skb)
static void
__rmnet_map_ingress_handler(struct sk_buff *skb,
struct rmnet_port *port)
struct rmnet_port *port,
u32 data_format)
{
struct rmnet_map_header *map_header = (void *)skb->data;
struct rmnet_endpoint *ep;
@ -63,8 +64,8 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
if (map_header->flags & MAP_CMD_FLAG) {
/* Packet contains a MAP command (not data) */
if (port->data_format & RMNET_FLAGS_INGRESS_MAP_COMMANDS)
return rmnet_map_command(skb, port);
if (data_format & RMNET_FLAGS_INGRESS_MAP_COMMANDS)
return rmnet_map_command(skb, port, data_format);
goto free_skb;
}
@ -82,7 +83,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
skb->dev = ep->egress_dev;
if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
(map_header->flags & MAP_NEXT_HEADER_FLAG)) {
if (rmnet_map_process_next_hdr_packet(skb, len))
goto free_skb;
@ -92,7 +93,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
/* Subtract MAP header */
skb_pull(skb, sizeof(*map_header));
rmnet_set_skb_proto(skb);
if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4 &&
if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4 &&
!rmnet_map_checksum_downlink_packet(skb, len + pad))
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
@ -110,6 +111,7 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
struct rmnet_port *port)
{
struct sk_buff *skbn;
u32 data_format;
if (skb->dev->type == ARPHRD_ETHER) {
if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_ATOMIC)) {
@ -120,14 +122,16 @@ rmnet_map_ingress_handler(struct sk_buff *skb,
skb_push(skb, ETH_HLEN);
}
if (port->data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
while ((skbn = rmnet_map_deaggregate(skb, port)) != NULL)
__rmnet_map_ingress_handler(skbn, port);
data_format = READ_ONCE(port->data_format);
if (data_format & RMNET_FLAGS_INGRESS_DEAGGREGATION) {
while ((skbn = rmnet_map_deaggregate(skb, data_format)) != NULL)
__rmnet_map_ingress_handler(skbn, port, data_format);
consume_skb(skb);
} else {
if (rmnet_map_validate_packet_len(skb, port))
__rmnet_map_ingress_handler(skb, port);
if (rmnet_map_validate_packet_len(skb, data_format))
__rmnet_map_ingress_handler(skb, port, data_format);
else
kfree_skb(skb);
}
@ -139,14 +143,16 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
{
int required_headroom, additional_header_len, csum_type = 0;
struct rmnet_map_header *map_header;
u32 data_format;
additional_header_len = 0;
required_headroom = sizeof(struct rmnet_map_header);
if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
data_format = READ_ONCE(port->data_format);
if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
} else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
} else if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
}
@ -161,7 +167,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
csum_type);
map_header = rmnet_map_add_map_header(skb, additional_header_len,
port, 0);
data_format, 0);
if (!map_header)
return -ENOMEM;

9
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
View File

@ -41,12 +41,13 @@ enum rmnet_map_commands {
#define RMNET_MAP_ADD_PAD_BYTES 1
struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
struct rmnet_port *port);
u32 data_format);
struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
int hdrlen,
struct rmnet_port *port,
u32 data_format,
int pad);
void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port,
u32 data_format);
int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
struct rmnet_port *port,
@ -59,6 +60,6 @@ void rmnet_map_tx_aggregate_init(struct rmnet_port *port);
void rmnet_map_tx_aggregate_exit(struct rmnet_port *port);
void rmnet_map_update_ul_agg_config(struct rmnet_port *port, u32 size,
u32 count, u32 time);
u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port);
u32 rmnet_map_validate_packet_len(struct sk_buff *skb, u32 data_format);
#endif /* _RMNET_MAP_H_ */

9
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
View File

@ -48,13 +48,13 @@ static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
static void rmnet_map_send_ack(struct sk_buff *skb,
unsigned char type,
struct rmnet_port *port)
u32 data_format)
{
struct rmnet_map_header *map_header = (void *)skb->data;
struct rmnet_map_control_command *cmd;
struct net_device *dev = skb->dev;
if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
skb_trim(skb,
skb->len - sizeof(struct rmnet_map_dl_csum_trailer));
@ -72,7 +72,8 @@ static void rmnet_map_send_ack(struct sk_buff *skb,
/* Process MAP command frame and send N/ACK message as appropriate. Message cmd
* name is decoded here and appropriate handler is called.
*/
void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port,
u32 data_format)
{
struct rmnet_map_header *map_header = (void *)skb->data;
struct rmnet_map_control_command *cmd;
@ -98,5 +99,5 @@ void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
break;
}
if (rc == RMNET_MAP_COMMAND_ACK)
rmnet_map_send_ack(skb, rc, port);
rmnet_map_send_ack(skb, rc, data_format);
}

14
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
View File

@ -294,7 +294,7 @@ static void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
*/
struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
int hdrlen,
struct rmnet_port *port,
u32 data_format,
int pad)
{
struct rmnet_map_header *map_header;
@ -306,7 +306,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
memset(map_header, 0, sizeof(struct rmnet_map_header));
/* Set next_hdr bit for csum offload packets */
if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
if (data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
map_header->flags |= MAP_NEXT_HEADER_FLAG;
if (pad == RMNET_MAP_NO_PAD_BYTES) {
@ -333,7 +333,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
return map_header;
}
u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
u32 rmnet_map_validate_packet_len(struct sk_buff *skb, u32 data_format)
{
struct rmnet_map_v5_csum_header *next_hdr = NULL;
struct rmnet_map_header *maph;
@ -351,9 +351,9 @@ u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
packet_len = ntohs(maph->pkt_len) + sizeof(*maph);
if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
if (data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
} else if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
} else if ((data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
!(maph->flags & MAP_CMD_FLAG)) {
/* Mapv5 data pkt without csum hdr is invalid */
if (!(maph->flags & MAP_NEXT_HEADER_FLAG))
@ -381,12 +381,12 @@ u32 rmnet_map_validate_packet_len(struct sk_buff *skb, struct rmnet_port *port)
* is responsible for freeing the original skb.
*/
struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
struct rmnet_port *port)
u32 data_format)
{
struct sk_buff *skbn;
u32 packet_len;
packet_len = rmnet_map_validate_packet_len(skb, port);
packet_len = rmnet_map_validate_packet_len(skb, data_format);
if (!packet_len)
return NULL;