Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 9 Apr 2024 17:08:54 -0700 | Subject | Re: [PATCH net] crypto: chtls: Fix possible null pointer dereferences | From | Jacob Keller <> |
| |
On 4/9/2024 2:05 AM, Aleksandr Mishin wrote: > In chtls_pass_accept_rpl() and chtls_select_mss() __sk_dst_get() may > return NULL which is later dereferenced. Fix this bug by adding NULL check. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: cc35c88ae4db ("crypto : chtls - CPL handler definition") > Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
This seems to do a lot more than simply adding a NULL check, though I guess thats because it needs to pass the return out somehow properly? I would have liked more explanation of the requires changes.
> --- > .../chelsio/inline_crypto/chtls/chtls_cm.c | 24 +++++++++++++------ > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c > index 6f6525983130..6d88cbc9fbb0 100644 > --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c > +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c > @@ -939,15 +939,15 @@ static void chtls_accept_rpl_arp_failure(void *handle, > sock_put(sk); > } > > -static unsigned int chtls_select_mss(const struct chtls_sock *csk, > +static bool chtls_select_mss(const struct chtls_sock *csk, > unsigned int pmtu, > - struct cpl_pass_accept_req *req) > + struct cpl_pass_accept_req *req, > + unsigned int *mtu_idx) > { > struct chtls_dev *cdev; > struct dst_entry *dst; > unsigned int tcpoptsz; > unsigned int iphdrsz; > - unsigned int mtu_idx; > struct tcp_sock *tp; > unsigned int mss; > struct sock *sk; > @@ -955,6 +955,9 @@ static unsigned int chtls_select_mss(const struct chtls_sock *csk, > mss = ntohs(req->tcpopt.mss); > sk = csk->sk; > dst = __sk_dst_get(sk); > + if (!dst) > + return false; > + > cdev = csk->cdev; > tp = tcp_sk(sk); > tcpoptsz = 0; > @@ -979,11 +982,11 @@ static unsigned int chtls_select_mss(const struct chtls_sock *csk, > tp->advmss = cxgb4_best_aligned_mtu(cdev->lldi->mtus, > iphdrsz + tcpoptsz, > tp->advmss - tcpoptsz, > - 8, &mtu_idx); > + 8, mtu_idx); > tp->advmss -= iphdrsz; > > inet_csk(sk)->icsk_pmtu_cookie = pmtu; > - return mtu_idx; > + return true; > } > > static unsigned int select_rcv_wscale(int space, int wscale_ok, int win_clamp) > @@ -1016,8 +1019,13 @@ static void chtls_pass_accept_rpl(struct sk_buff *skb, > struct sock *sk; > u32 opt2, hlen; > u64 opt0; > + struct dst_entry *dst; > > sk = skb->sk; > + dst = __sk_dst_get(sk); > + if (!dst) > + return; > + > tp = tcp_sk(sk); > csk = sk->sk_user_data; > csk->tid = tid; > @@ -1029,8 +1037,10 @@ static void chtls_pass_accept_rpl(struct sk_buff *skb, > > OPCODE_TID(rpl5) = cpu_to_be32(MK_OPCODE_TID(CPL_PASS_ACCEPT_RPL, > csk->tid)); > - csk->mtu_idx = chtls_select_mss(csk, dst_mtu(__sk_dst_get(sk)), > - req); > + if (!chtls_select_mss(csk, dst_mtu(dst), > + req, &csk->mtu_idx)) > + return; > + > opt0 = TCAM_BYPASS_F | > WND_SCALE_V(RCV_WSCALE(tp)) | > MSS_IDX_V(csk->mtu_idx) |
| ![\](/images/icornerr.gif) |