| Message ID | 20250401124018.4763-1-mowenroot@163.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] netlabel: Fix NULL pointer exception caused by CALIPSO on IPv4 sockets | expand |
On Tue, Apr 1, 2025 at 8:40 AM Debin Zhu <mowenroot@163.com> wrote: > > When calling netlbl_conn_setattr(), addr->sa_family is used > to determine the function behavior. If sk is an IPv4 socket, > but the connect function is called with an IPv6 address, > the function calipso_sock_setattr() is triggered. > Inside this function, the following code is executed: > > sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL; > > Since sk is an IPv4 socket, pinet6 is NULL, leading to a > null pointer dereference. > > This patch fixes the issue by checking if inet6_sk(sk) > returns a NULL pointer before accessing pinet6. > > Fixes: ceba1832b1b2("calipso: Set the calipso socket label to match the secattr.") > Signed-off-by: Debin Zhu <mowenroot@163.com> > Signed-off-by: Bitao Ouyang <1985755126@qq.com> > Acked-by: Paul Moore <paul@paul-moore.com> As a FYI in case people aren't seeing the previous messages in this thread, I did ACK an earlier version of this patch, and while there have been some changes to the code, they are largely superficial and my ACK still applies. The commit description looks okay to me. Paolo, do you plan to take this via the netdev tree or would you like me to take this? Regardless, the patch should also be marked for stable. > --- > net/ipv6/calipso.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c > index dbcea9fee..a8a8736df 100644 > --- a/net/ipv6/calipso.c > +++ b/net/ipv6/calipso.c > @@ -1072,8 +1072,13 @@ static int calipso_sock_getattr(struct sock *sk, > struct ipv6_opt_hdr *hop; > int opt_len, len, ret_val = -ENOMSG, offset; > unsigned char *opt; > - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); > + struct ipv6_pinfo *pinfo = inet6_sk(sk); > + struct ipv6_txoptions *txopts; > > + if (!pinfo) > + return -EAFNOSUPPORT; > + > + txopts = txopt_get(pinfo); > if (!txopts || !txopts->hopopt) > goto done; > > @@ -1125,8 +1130,13 @@ static int calipso_sock_setattr(struct sock *sk, > { > int ret_val; > struct ipv6_opt_hdr *old, *new; > - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); > - > + struct ipv6_pinfo *pinfo = inet6_sk(sk); > + struct ipv6_txoptions *txopts; > + > + if (!pinfo) > + return -EAFNOSUPPORT; > + > + txopts = txopt_get(pinfo); > old = NULL; > if (txopts) > old = txopts->hopopt; > @@ -1153,8 +1163,13 @@ static int calipso_sock_setattr(struct sock *sk, > static void calipso_sock_delattr(struct sock *sk) > { > struct ipv6_opt_hdr *new_hop; > - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); > + struct ipv6_pinfo *pinfo = inet6_sk(sk); > + struct ipv6_txoptions *txopts; > > + if (!pinfo) > + return; > + > + txopts = txopt_get(pinfo); > if (!txopts || !txopts->hopopt) > goto done; > > -- > 2.34.1
On Tue, Apr 01, 2025 at 08:40:18PM +0800, Debin Zhu wrote: > When calling netlbl_conn_setattr(), addr->sa_family is used > to determine the function behavior. If sk is an IPv4 socket, > but the connect function is called with an IPv6 address, > the function calipso_sock_setattr() is triggered. > Inside this function, the following code is executed: > > sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL; > > Since sk is an IPv4 socket, pinet6 is NULL, leading to a > null pointer dereference. > > This patch fixes the issue by checking if inet6_sk(sk) > returns a NULL pointer before accessing pinet6. > > Fixes: ceba1832b1b2("calipso: Set the calipso socket label to match the secattr.") There is probably no need to repost for this, but there is a missing space in the Fixes tag. It should be like this: Fixes: ceba1832b1b2 ("calipso: Set the calipso socket label to match the secattr.") > Signed-off-by: Debin Zhu <mowenroot@163.com> > Signed-off-by: Bitao Ouyang <1985755126@qq.com> > Acked-by: Paul Moore <paul@paul-moore.com> ...
On Wed, Apr 2, 2025 at 5:36 AM Simon Horman <horms@kernel.org> wrote: > On Tue, Apr 01, 2025 at 08:40:18PM +0800, Debin Zhu wrote: > > When calling netlbl_conn_setattr(), addr->sa_family is used > > to determine the function behavior. If sk is an IPv4 socket, > > but the connect function is called with an IPv6 address, > > the function calipso_sock_setattr() is triggered. > > Inside this function, the following code is executed: > > > > sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL; > > > > Since sk is an IPv4 socket, pinet6 is NULL, leading to a > > null pointer dereference. > > > > This patch fixes the issue by checking if inet6_sk(sk) > > returns a NULL pointer before accessing pinet6. > > > > Fixes: ceba1832b1b2("calipso: Set the calipso socket label to match the secattr.") > > There is probably no need to repost for this, but > there is a missing space in the Fixes tag. It should be like this: > > Fixes: ceba1832b1b2 ("calipso: Set the calipso socket label to match the secattr.") Thanks. Not sure if the netdev folks are going to pick this up or if I'll end up taking it, but if I end up taking it I'll update the tag while merging. > > Signed-off-by: Debin Zhu <mowenroot@163.com> > > Signed-off-by: Bitao Ouyang <1985755126@qq.com> > > Acked-by: Paul Moore <paul@paul-moore.com>
diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c index dbcea9fee..a8a8736df 100644 --- a/net/ipv6/calipso.c +++ b/net/ipv6/calipso.c @@ -1072,8 +1072,13 @@ static int calipso_sock_getattr(struct sock *sk, struct ipv6_opt_hdr *hop; int opt_len, len, ret_val = -ENOMSG, offset; unsigned char *opt; - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); + struct ipv6_pinfo *pinfo = inet6_sk(sk); + struct ipv6_txoptions *txopts; + if (!pinfo) + return -EAFNOSUPPORT; + + txopts = txopt_get(pinfo); if (!txopts || !txopts->hopopt) goto done; @@ -1125,8 +1130,13 @@ static int calipso_sock_setattr(struct sock *sk, { int ret_val; struct ipv6_opt_hdr *old, *new; - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); - + struct ipv6_pinfo *pinfo = inet6_sk(sk); + struct ipv6_txoptions *txopts; + + if (!pinfo) + return -EAFNOSUPPORT; + + txopts = txopt_get(pinfo); old = NULL; if (txopts) old = txopts->hopopt; @@ -1153,8 +1163,13 @@ static int calipso_sock_setattr(struct sock *sk, static void calipso_sock_delattr(struct sock *sk) { struct ipv6_opt_hdr *new_hop; - struct ipv6_txoptions *txopts = txopt_get(inet6_sk(sk)); + struct ipv6_pinfo *pinfo = inet6_sk(sk); + struct ipv6_txoptions *txopts; + if (!pinfo) + return; + + txopts = txopt_get(pinfo); if (!txopts || !txopts->hopopt) goto done;
When calling netlbl_conn_setattr(), addr->sa_family is used to determine the function behavior. If sk is an IPv4 socket, but the connect function is called with an IPv6 address, the function calipso_sock_setattr() is triggered. Inside this function, the following code is executed: sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL; Since sk is an IPv4 socket, pinet6 is NULL, leading to a null pointer dereference. This patch fixes the issue by checking if inet6_sk(sk) returns a NULL pointer before accessing pinet6. Fixes: ceba1832b1b2("calipso: Set the calipso socket label to match the secattr.") Signed-off-by: Debin Zhu <mowenroot@163.com> Signed-off-by: Bitao Ouyang <1985755126@qq.com> Acked-by: Paul Moore <paul@paul-moore.com> --- net/ipv6/calipso.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)