net: Mark the sk parameter of routing functions as 'const'.

PATCH Applied Guillaume Nault 5 participants 14 messages Jul 11, 2023 – Jul 18, 2023

Subsystems (5)

Networking Network drivers SELinux security module IPv4/IPv6 +1 more subsystems

lore

Share & tools

lore

Guillaume Nault

author

2023-07-11 13:06 UTC #0 lore raw

cover net: Mark the sk parameter of routing functions as 'const'.

The sk_getsecid security hook prevents the use of a const sk pointer in several routing functions. Since this hook should only read sk data, make its sk argument const (patch 1), then constify the sk parameter of various routing functions (patches 2-4).

Build-tested with make allmodconfig.

Guillaume Nault (4):
security: Constify sk in the sk_getsecid hook.
ipv4: Constify the sk parameter of ip_route_output_*().
ipv6: Constify the sk parameter of several helper functions.
pptp: Constify the po parameter of pptp_route_output().

 drivers/net/ppp/pptp.c        |  4 ++--
 include/linux/icmpv6.h        | 10 ++++------
 include/linux/lsm_hook_defs.h |  2 +-
 include/linux/security.h      |  5 +++--
 include/net/route.h           |  6 +++---
 net/ipv6/datagram.c           |  7 ++++---
 net/ipv6/icmp.c               |  6 ++----
 net/ipv6/mcast.c              |  8 +++-----
 security/security.c           |  2 +-
 security/selinux/hooks.c      |  4 ++--
 10 files changed, 25 insertions(+), 29 deletions(-)

Signature · 1 line

2.39.2

Guillaume Nault

author

2023-07-11 13:06 UTC #1 lore raw

PATCH 1/4 security: Constify sk in the sk_getsecid hook.

The sk_getsecid hook shouldn't need to modify its socket argument. Make it const so that callers of security_sk_classify_flow() can use a const struct sock *.

Signed-off-by Guillaume Nault <[email protected]>

---
 include/linux/lsm_hook_defs.h | 2 +-
 include/linux/security.h      | 5 +++--
 security/security.c           | 2 +-
 security/selinux/hooks.c      | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

include/linux/lsm_hook_defs.h +1 -1

index 7308a1a7599b..4f2621e87634 100644
@@ -316,7 +316,7 @@ LSM_HOOK(int, 0, sk_alloc_security, struct sock *sk, int family, gfp_t priority)
 LSM_HOOK(void, LSM_RET_VOID, sk_free_security, struct sock *sk)
 LSM_HOOK(void, LSM_RET_VOID, sk_clone_security, const struct sock *sk,
 	 struct sock *newsk)
-LSM_HOOK(void, LSM_RET_VOID, sk_getsecid, struct sock *sk, u32 *secid)
+LSM_HOOK(void, LSM_RET_VOID, sk_getsecid, const struct sock *sk, u32 *secid)
 LSM_HOOK(void, LSM_RET_VOID, sock_graft, struct sock *sk, struct socket *parent)
 LSM_HOOK(int, 0, inet_conn_request, const struct sock *sk, struct sk_buff *skb,
 	 struct request_sock *req)

include/linux/security.h +3 -2

index 32828502f09e..994cf099d9ac 100644
@@ -1439,7 +1439,8 @@ int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u
 int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
 void security_sk_free(struct sock *sk);
 void security_sk_clone(const struct sock *sk, struct sock *newsk);
-void security_sk_classify_flow(struct sock *sk, struct flowi_common *flic);
+void security_sk_classify_flow(const struct sock *sk,
+			       struct flowi_common *flic);
 void security_req_classify_flow(const struct request_sock *req,
 				struct flowi_common *flic);
 void security_sock_graft(struct sock*sk, struct socket *parent);

@@ -1597,7 +1598,7 @@ static inline void security_sk_clone(const struct sock *sk, struct sock *newsk)
 {
 }
 
-static inline void security_sk_classify_flow(struct sock *sk,
+static inline void security_sk_classify_flow(const struct sock *sk,
 					     struct flowi_common *flic)
 {
 }

security/security.c +1 -1

index b720424ca37d..2dfc7b9f6ed9 100644
@@ -4396,7 +4396,7 @@ void security_sk_clone(const struct sock *sk, struct sock *newsk)
 }
 EXPORT_SYMBOL(security_sk_clone);
 
-void security_sk_classify_flow(struct sock *sk, struct flowi_common *flic)
+void security_sk_classify_flow(const struct sock *sk, struct flowi_common *flic)
 {
 	call_void_hook(sk_getsecid, sk, &flic->flowic_secid);
 }

security/selinux/hooks.c +2 -2

index d06e350fedee..2bdc48dd8670 100644
@@ -5167,12 +5167,12 @@ static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
 	selinux_netlbl_sk_security_reset(newsksec);
 }
 
-static void selinux_sk_getsecid(struct sock *sk, u32 *secid)
+static void selinux_sk_getsecid(const struct sock *sk, u32 *secid)
 {
 	if (!sk)
 		*secid = SECINITSID_ANY_SOCKET;
 	else {
-		struct sk_security_struct *sksec = sk->sk_security;
+		const struct sk_security_struct *sksec = sk->sk_security;
 
 		*secid = sksec->sid;
 	}

Signature · 1 line

2.39.2

Simon Horman

Reviewed-by on v1 1/4

2023-07-13 09:32 UTC #2 lore raw

Re: [PATCH net-next 1/4] security: Constify sk in the sk_getsecid hook.

Quoted 5 lines · Guillaume Nault

The sk_getsecid hook shouldn't need to modify its socket argument. Make it const so that callers of security_sk_classify_flow() can use a const struct sock *.
Signed-off-by Guillaume Nault <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Paul Moore

maintainer

2023-07-17 16:05 UTC #3 lore raw

Re: [PATCH net-next 1/4] security: Constify sk in the sk_getsecid hook.

In reply to Guillaume Nault

Quoted 5 lines · Guillaume Nault

The sk_getsecid hook shouldn't need to modify its socket argument. Make it const so that callers of security_sk_classify_flow() can use a const struct sock *.

Quoted 7 lines

Signed-off-by Guillaume Nault <[email protected]>

---
 include/linux/lsm_hook_defs.h | 2 +-
 include/linux/security.h      | 5 +++--
 security/security.c           | 2 +-
 security/selinux/hooks.c      | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

Thanks Guillaume, this looks good to me. I had limited network access last week and was only monitoring my email for urgent issues, but from what I can tell it looks like this was picked up in the netdev tree so I'll leave it alone, but if anything changes let me know and I'll merge it via the LSM tree.

Signature · 1 line

paul-moore.com

Guillaume Nault

author

2023-07-18 12:31 UTC #4 lore raw

Re: [PATCH net-next 1/4] security: Constify sk in the sk_getsecid hook.

Quoted 14 lines · Paul Moore

Quoted 5 lines · Guillaume Nault
The sk_getsecid hook shouldn't need to modify its socket argument. Make it const so that callers of security_sk_classify_flow() can use a const struct sock *.
Quoted 7 lines
Signed-off-by Guillaume Nault <[email protected]>
---
 include/linux/lsm_hook_defs.h | 2 +-
 include/linux/security.h      | 5 +++--
 security/security.c           | 2 +-
 security/selinux/hooks.c      | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

Quoted 5 lines

Thanks Guillaume, this looks good to me. I had limited network access last week and was only monitoring my email for urgent issues, but from what I can tell it looks like this was picked up in the netdev tree so I'll leave it alone, but if anything changes let me know and I'll merge it via the LSM tree.

Thanks Paul, this series has indeed been applied to the networking tree. So no special action is needed.

Quoted 3 lines

Signature · 1 line
paul-moore.com

Guillaume Nault

author

2023-07-11 13:06 UTC #5 lore raw

PATCH 2/4 ipv4: Constify the sk parameter of ip_route_output_*().

In reply to Guillaume Nault

These functions don't need to modify the socket, so let's allow the callers to pass a const struct sock *.

Signed-off-by Guillaume Nault <[email protected]>

---
 include/net/route.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

include/net/route.h +3 -3

index 5a5c726472bd..d8d150155195 100644
@@ -163,7 +163,7 @@ static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
 }
 
 static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi4 *fl4,
-						   struct sock *sk,
+						   const struct sock *sk,
 						   __be32 daddr, __be32 saddr,
 						   __be16 dport, __be16 sport,
 						   __u8 proto, __u8 tos, int oif)

@@ -309,7 +309,7 @@ static inline void ip_route_connect_init(struct flowi4 *fl4, __be32 dst,
 static inline struct rtable *ip_route_connect(struct flowi4 *fl4, __be32 dst,
 					      __be32 src, int oif, u8 protocol,
 					      __be16 sport, __be16 dport,
-					      struct sock *sk)
+					      const struct sock *sk)
 {
 	struct net *net = sock_net(sk);
 	struct rtable *rt;

@@ -330,7 +330,7 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4, __be32 dst,
 static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable *rt,
 					       __be16 orig_sport, __be16 orig_dport,
 					       __be16 sport, __be16 dport,
-					       struct sock *sk)
+					       const struct sock *sk)
 {
 	if (sport != orig_sport || dport != orig_dport) {
 		fl4->fl4_dport = dport;

Signature · 1 line

2.39.2

Simon Horman

Reviewed-by on v1 2/4

2023-07-13 09:33 UTC #6 lore raw

Re: [PATCH net-next 2/4] ipv4: Constify the sk parameter of ip_route_output_*().

Quoted 4 lines · Guillaume Nault

These functions don't need to modify the socket, so let's allow the callers to pass a const struct sock *.
Signed-off-by Guillaume Nault <[email protected]>

Reviewed-by Simon Horman <[email protected]>

David Ahern

maintainer Reviewed-by on v1 2/4

2023-07-13 21:33 UTC #7 lore raw

Re: [PATCH net-next 2/4] ipv4: Constify the sk parameter of ip_route_output_*().

In reply to Guillaume Nault

Quoted 8 lines · Guillaume Nault

These functions don't need to modify the socket, so let's allow the callers to pass a const struct sock *.
Signed-off-by Guillaume Nault <[email protected]>
---
 include/net/route.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by David Ahern <[email protected]>

Guillaume Nault

author

2023-07-11 13:06 UTC #8 lore raw

PATCH 3/4 ipv6: Constify the sk parameter of several helper functions.

In reply to Guillaume Nault

icmpv6_flow_init(), ip6_datagram_flow_key_init() and ip6_mc_hdr() don't need to modify their sk argument. Make that explicit using const.

Signed-off-by Guillaume Nault <[email protected]>

---
 include/linux/icmpv6.h | 10 ++++------
 net/ipv6/datagram.c    |  7 ++++---
 net/ipv6/icmp.c        |  6 ++----
 net/ipv6/mcast.c       |  8 +++-----
 4 files changed, 13 insertions(+), 18 deletions(-)

include/linux/icmpv6.h +4 -6

index db0f4fcfdaf4..e3b3b0fa2a8f 100644
@@ -85,12 +85,10 @@ extern void				icmpv6_param_prob_reason(struct sk_buff *skb,
 
 struct flowi6;
 struct in6_addr;
-extern void				icmpv6_flow_init(struct sock *sk,
-							 struct flowi6 *fl6,
-							 u8 type,
-							 const struct in6_addr *saddr,
-							 const struct in6_addr *daddr,
-							 int oif);
+
+void icmpv6_flow_init(const struct sock *sk, struct flowi6 *fl6, u8 type,
+		      const struct in6_addr *saddr,
+		      const struct in6_addr *daddr, int oif);
 
 static inline void icmpv6_param_prob(struct sk_buff *skb, u8 code, int pos)
 {

net/ipv6/datagram.c +4 -3

index 9b6818453afe..d80d6024cafa 100644
@@ -38,10 +38,11 @@ static bool ipv6_mapped_addr_any(const struct in6_addr *a)
 	return ipv6_addr_v4mapped(a) && (a->s6_addr32[3] == 0);
 }
 
-static void ip6_datagram_flow_key_init(struct flowi6 *fl6, struct sock *sk)
+static void ip6_datagram_flow_key_init(struct flowi6 *fl6,
+				       const struct sock *sk)
 {
-	struct inet_sock *inet = inet_sk(sk);
-	struct ipv6_pinfo *np = inet6_sk(sk);
+	const struct inet_sock *inet = inet_sk(sk);
+	const struct ipv6_pinfo *np = inet6_sk(sk);
 	int oif = sk->sk_bound_dev_if;
 
 	memset(fl6, 0, sizeof(*fl6));

net/ipv6/icmp.c +2 -4

index 9edf1f45b1ed..988d21166837 100644
@@ -1031,11 +1031,9 @@ static int icmpv6_rcv(struct sk_buff *skb)
 	return 0;
 }
 
-void icmpv6_flow_init(struct sock *sk, struct flowi6 *fl6,
-		      u8 type,
+void icmpv6_flow_init(const struct sock *sk, struct flowi6 *fl6, u8 type,
 		      const struct in6_addr *saddr,
-		      const struct in6_addr *daddr,
-		      int oif)
+		      const struct in6_addr *daddr, int oif)
 {
 	memset(fl6, 0, sizeof(*fl6));
 	fl6->saddr = *saddr;

net/ipv6/mcast.c +3 -5

index 714cdc9e2b8e..5ce25bcb9974 100644
@@ -1699,11 +1699,9 @@ mld_scount(struct ifmcaddr6 *pmc, int type, int gdeleted, int sdeleted)
 	return scount;
 }
 
-static void ip6_mc_hdr(struct sock *sk, struct sk_buff *skb,
-		       struct net_device *dev,
-		       const struct in6_addr *saddr,
-		       const struct in6_addr *daddr,
-		       int proto, int len)
+static void ip6_mc_hdr(const struct sock *sk, struct sk_buff *skb,
+		       struct net_device *dev, const struct in6_addr *saddr,
+		       const struct in6_addr *daddr, int proto, int len)
 {
 	struct ipv6hdr *hdr;

Signature · 1 line

2.39.2

Simon Horman

Reviewed-by on v1 3/4

2023-07-13 09:33 UTC #9 lore raw

Re: [PATCH net-next 3/4] ipv6: Constify the sk parameter of several helper functions.

Quoted 4 lines · Guillaume Nault

icmpv6_flow_init(), ip6_datagram_flow_key_init() and ip6_mc_hdr() don't need to modify their sk argument. Make that explicit using const.
Signed-off-by Guillaume Nault <[email protected]>

Reviewed-by Simon Horman <[email protected]>

David Ahern

maintainer Reviewed-by on v1 3/4

2023-07-13 21:34 UTC #10 lore raw

Re: [PATCH net-next 3/4] ipv6: Constify the sk parameter of several helper functions.

In reply to Guillaume Nault

Quoted 11 lines · Guillaume Nault

icmpv6_flow_init(), ip6_datagram_flow_key_init() and ip6_mc_hdr() don't need to modify their sk argument. Make that explicit using const.
Signed-off-by Guillaume Nault <[email protected]>
---
 include/linux/icmpv6.h | 10 ++++------
 net/ipv6/datagram.c    |  7 ++++---
 net/ipv6/icmp.c        |  6 ++----
 net/ipv6/mcast.c       |  8 +++-----
 4 files changed, 13 insertions(+), 18 deletions(-)

Reviewed-by David Ahern <[email protected]>

Guillaume Nault

author

2023-07-11 13:06 UTC #11 lore raw

PATCH 4/4 pptp: Constify the po parameter of pptp_route_output().

In reply to Guillaume Nault

Make it explicit that this function doesn't modify the socket passed as parameter.

Signed-off-by Guillaume Nault <[email protected]>

---
 drivers/net/ppp/pptp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ppp/pptp.c +2 -2

index 32183f24e63f..57d38b27812d 100644
@@ -129,10 +129,10 @@ static void del_chan(struct pppox_sock *sock)
 	spin_unlock(&chan_lock);
 }
 
-static struct rtable *pptp_route_output(struct pppox_sock *po,
+static struct rtable *pptp_route_output(const struct pppox_sock *po,
 					struct flowi4 *fl4)
 {
-	struct sock *sk = &po->sk;
+	const struct sock *sk = &po->sk;
 	struct net *net;
 
 	net = sock_net(sk);

Signature · 1 line

2.39.2

Simon Horman

Reviewed-by on v1 4/4

2023-07-13 09:33 UTC #12 lore raw

Re: [PATCH net-next 4/4] pptp: Constify the po parameter of pptp_route_output().

Quoted 4 lines · Guillaume Nault

Make it explicit that this function doesn't modify the socket passed as parameter.
Signed-off-by Guillaume Nault <[email protected]>

Reviewed-by Simon Horman <[email protected]>

[email protected]

CI Applied

2023-07-14 07:40 UTC #13 lore raw

In reply to Guillaume Nault

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Quoted 8 lines · On Tue, 11 Jul 2023 15:06:00 +0200 you wrote:

The sk_getsecid security hook prevents the use of a const sk pointer in several routing functions. Since this hook should only read sk data, make its sk argument const (patch 1), then constify the sk parameter of various routing functions (patches 2-4).
Build-tested with make allmodconfig.
[...]

Here is the summary with links:
- [net-next,1/4] security: Constify sk in the sk_getsecid hook.

    https://git.kernel.org/netdev/net-next/c/5b52ad34f948
  - [net-next,2/4] ipv4: Constify the sk parameter of ip_route_output_*().
    https://git.kernel.org/netdev/net-next/c/8d6eba33a272
  - [net-next,3/4] ipv6: Constify the sk parameter of several helper functions.
    https://git.kernel.org/netdev/net-next/c/5bc67a854cb4
  - [net-next,4/4] pptp: Constify the po parameter of pptp_route_output().
    https://git.kernel.org/netdev/net-next/c/dc4c399d215d

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html