From e3af3d3c5b26c33a7950e34e137584f6056c4319 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 9 Aug 2024 16:54:02 -0700 Subject: [PATCH 1/5] ipv4: Check !in_dev earlier for ioctl(SIOCSIFADDR). dev->ip_ptr could be NULL if we set an invalid MTU. Even then, if we issue ioctl(SIOCSIFADDR) for a new IPv4 address, devinet_ioctl() allocates struct in_ifaddr and fails later in inet_set_ifa() because in_dev is NULL. Let's move the check earlier. Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20240809235406.50187-2-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/ipv4/devinet.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index d96f3e452fef..ddab15116454 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -574,10 +574,6 @@ static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa) ASSERT_RTNL(); - if (!in_dev) { - inet_free_ifa(ifa); - return -ENOBUFS; - } ipv4_devconf_setall(in_dev); neigh_parms_data_state_setall(in_dev->arp_parms); if (ifa->ifa_dev != in_dev) { @@ -1184,6 +1180,8 @@ int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr) if (!ifa) { ret = -ENOBUFS; + if (!in_dev) + break; ifa = inet_alloc_ifa(); if (!ifa) break; From 6e701eb914124cf260a8b9a4350740f9a7407fc7 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 9 Aug 2024 16:54:03 -0700 Subject: [PATCH 2/5] ipv4: Set ifa->ifa_dev in inet_alloc_ifa(). When a new IPv4 address is assigned via ioctl(SIOCSIFADDR), inet_set_ifa() sets ifa->ifa_dev if it's different from in_dev passed as an argument. In this case, ifa is always a newly allocated object, and ifa->ifa_dev is NULL. inet_set_ifa() can be called for an existing reused ifa, then, this check is always false. Let's set ifa_dev in inet_alloc_ifa() and remove the check in inet_set_ifa(). Now, inet_alloc_ifa() is symmetric with inet_rcu_free_ifa(). Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20240809235406.50187-3-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/ipv4/devinet.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index ddab15116454..9f4add07e67d 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -216,9 +216,18 @@ static void devinet_sysctl_unregister(struct in_device *idev) /* Locks all the inet devices. */ -static struct in_ifaddr *inet_alloc_ifa(void) +static struct in_ifaddr *inet_alloc_ifa(struct in_device *in_dev) { - return kzalloc(sizeof(struct in_ifaddr), GFP_KERNEL_ACCOUNT); + struct in_ifaddr *ifa; + + ifa = kzalloc(sizeof(*ifa), GFP_KERNEL_ACCOUNT); + if (!ifa) + return NULL; + + in_dev_hold(in_dev); + ifa->ifa_dev = in_dev; + + return ifa; } static void inet_rcu_free_ifa(struct rcu_head *head) @@ -576,11 +585,7 @@ static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa) ipv4_devconf_setall(in_dev); neigh_parms_data_state_setall(in_dev->arp_parms); - if (ifa->ifa_dev != in_dev) { - WARN_ON(ifa->ifa_dev); - in_dev_hold(in_dev); - ifa->ifa_dev = in_dev; - } + if (ipv4_is_loopback(ifa->ifa_local)) ifa->ifa_scope = RT_SCOPE_HOST; return inet_insert_ifa(ifa); @@ -871,7 +876,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh, if (!in_dev) goto errout; - ifa = inet_alloc_ifa(); + ifa = inet_alloc_ifa(in_dev); if (!ifa) /* * A potential indev allocation can be left alive, it stays @@ -881,7 +886,6 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh, ipv4_devconf_setall(in_dev); neigh_parms_data_state_setall(in_dev->arp_parms); - in_dev_hold(in_dev); if (!tb[IFA_ADDRESS]) tb[IFA_ADDRESS] = tb[IFA_LOCAL]; @@ -892,8 +896,6 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh, ifa->ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags; ifa->ifa_scope = ifm->ifa_scope; - ifa->ifa_dev = in_dev; - ifa->ifa_local = nla_get_in_addr(tb[IFA_LOCAL]); ifa->ifa_address = nla_get_in_addr(tb[IFA_ADDRESS]); @@ -1182,7 +1184,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr) ret = -ENOBUFS; if (!in_dev) break; - ifa = inet_alloc_ifa(); + ifa = inet_alloc_ifa(in_dev); if (!ifa) break; INIT_HLIST_NODE(&ifa->hash); @@ -1584,7 +1586,7 @@ static int inetdev_event(struct notifier_block *this, unsigned long event, if (!inetdev_valid_mtu(dev->mtu)) break; if (dev->flags & IFF_LOOPBACK) { - struct in_ifaddr *ifa = inet_alloc_ifa(); + struct in_ifaddr *ifa = inet_alloc_ifa(in_dev); if (ifa) { INIT_HLIST_NODE(&ifa->hash); @@ -1592,8 +1594,6 @@ static int inetdev_event(struct notifier_block *this, unsigned long event, ifa->ifa_address = htonl(INADDR_LOOPBACK); ifa->ifa_prefixlen = 8; ifa->ifa_mask = inet_make_mask(8); - in_dev_hold(in_dev); - ifa->ifa_dev = in_dev; ifa->ifa_scope = RT_SCOPE_HOST; memcpy(ifa->ifa_label, dev->name, IFNAMSIZ); set_ifa_lifetime(ifa, INFINITY_LIFE_TIME, From ecdae5168460f7260c0f8dad069aa06f9ba0706e Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 9 Aug 2024 16:54:04 -0700 Subject: [PATCH 3/5] ipv4: Remove redundant !ifa->ifa_dev check. Now, ifa_dev is only set in inet_alloc_ifa() and never NULL after ifa gets visible. Let's remove the unneeded NULL check for ifa->ifa_dev. Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20240809235406.50187-4-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c | 5 ++--- drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +- net/ipv4/devinet.c | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index ed24d6af7487..9cff0a8ffb2c 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -3185,8 +3185,7 @@ netxen_list_config_ip(struct netxen_adapter *adapter, struct list_head *head; bool ret = false; - dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL; - + dev = ifa->ifa_dev->dev; if (dev == NULL) goto out; @@ -3379,7 +3378,7 @@ netxen_inetaddr_event(struct notifier_block *this, struct in_ifaddr *ifa = (struct in_ifaddr *)ptr; unsigned long ip_event; - dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL; + dev = ifa->ifa_dev->dev; ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN; recheck: if (dev == NULL) diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c index 90df4a0909fa..b3588a1ebc25 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c @@ -4146,7 +4146,7 @@ qlcnic_inetaddr_event(struct notifier_block *this, struct in_ifaddr *ifa = (struct in_ifaddr *)ptr; - dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL; + dev = ifa->ifa_dev->dev; recheck: if (dev == NULL) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index 9f4add07e67d..baf036bfad76 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -234,8 +234,7 @@ static void inet_rcu_free_ifa(struct rcu_head *head) { struct in_ifaddr *ifa = container_of(head, struct in_ifaddr, rcu_head); - if (ifa->ifa_dev) - in_dev_put(ifa->ifa_dev); + in_dev_put(ifa->ifa_dev); kfree(ifa); } From 100465a91a900699db9c64fb55063affd15c4362 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 9 Aug 2024 16:54:05 -0700 Subject: [PATCH 4/5] ipv4: Initialise ifa->hash in inet_alloc_ifa(). Whenever ifa is allocated, we call INIT_HLIST_NODE(&ifa->hash). Let's move it to inet_alloc_ifa(). Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20240809235406.50187-5-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/ipv4/devinet.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index baf036bfad76..b5d2a9fd46c7 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -227,6 +227,8 @@ static struct in_ifaddr *inet_alloc_ifa(struct in_device *in_dev) in_dev_hold(in_dev); ifa->ifa_dev = in_dev; + INIT_HLIST_NODE(&ifa->hash); + return ifa; } @@ -889,7 +891,6 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh, if (!tb[IFA_ADDRESS]) tb[IFA_ADDRESS] = tb[IFA_LOCAL]; - INIT_HLIST_NODE(&ifa->hash); ifa->ifa_prefixlen = ifm->ifa_prefixlen; ifa->ifa_mask = inet_make_mask(ifm->ifa_prefixlen); ifa->ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : @@ -1186,7 +1187,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr) ifa = inet_alloc_ifa(in_dev); if (!ifa) break; - INIT_HLIST_NODE(&ifa->hash); + if (colon) memcpy(ifa->ifa_label, ifr->ifr_name, IFNAMSIZ); else @@ -1588,7 +1589,6 @@ static int inetdev_event(struct notifier_block *this, unsigned long event, struct in_ifaddr *ifa = inet_alloc_ifa(in_dev); if (ifa) { - INIT_HLIST_NODE(&ifa->hash); ifa->ifa_local = ifa->ifa_address = htonl(INADDR_LOOPBACK); ifa->ifa_prefixlen = 8; From de67763cbdbba8149eeb5d45ad0c6834f7c7b352 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 9 Aug 2024 16:54:06 -0700 Subject: [PATCH 5/5] ip: Move INFINITY_LIFE_TIME to addrconf.h. INFINITY_LIFE_TIME is the common value used in IPv4 and IPv6 but defined in both .c files. Also, 0xffffffff used in addrconf_timeout_fixup() is INFINITY_LIFE_TIME. Let's move INFINITY_LIFE_TIME's definition to addrconf.h Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20240809235406.50187-6-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- include/net/addrconf.h | 4 +++- net/ipv4/devinet.c | 2 -- net/ipv6/addrconf.c | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/net/addrconf.h b/include/net/addrconf.h index b18e81f0c9e1..c8ed31828db3 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -187,10 +187,12 @@ static inline int addrconf_ifid_eui48(u8 *eui, struct net_device *dev) return 0; } +#define INFINITY_LIFE_TIME 0xFFFFFFFF + static inline unsigned long addrconf_timeout_fixup(u32 timeout, unsigned int unit) { - if (timeout == 0xffffffff) + if (timeout == INFINITY_LIFE_TIME) return ~0UL; /* diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index b5d2a9fd46c7..61be85154dd1 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -703,8 +703,6 @@ errout: return err; } -#define INFINITY_LIFE_TIME 0xFFFFFFFF - static void check_lifetime(struct work_struct *work) { unsigned long now, next, next_sec, next_sched; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index c87d008aefa4..04ee75af2f6b 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -92,8 +92,6 @@ #include #include -#define INFINITY_LIFE_TIME 0xFFFFFFFF - #define IPV6_MAX_STRLEN \ sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")