* [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-22 15:19 ` Peilin Ye 0 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-22 15:19 UTC (permalink / raw) To: Joerg Reuter, Ralf Baechle Cc: Peilin Ye, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` equals to 7 or 8. Fix it. This issue has been reported as a KMSAN uninit-value bug, because in such a case, ax25_connect() reaches into the uninitialized portion of the `struct sockaddr_storage` statically allocated in __sys_connect(). It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because `addr_len` is guaranteed to be less than or equal to `sizeof(struct full_sockaddr_ax25)`. Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> --- net/ax25/af_ax25.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index fd91cd34f25e..ef5bf116157a 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, if (addr_len > sizeof(struct sockaddr_ax25) && fsa->fsa_ax25.sax25_ndigis != 0) { /* Valid number of digipeaters ? */ - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { + if (fsa->fsa_ax25.sax25_ndigis < 1 || + addr_len < sizeof(struct sockaddr_ax25) + + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { err = -EINVAL; goto out_release; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-22 15:19 ` Peilin Ye 0 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-22 15:19 UTC (permalink / raw) To: Joerg Reuter, Ralf Baechle Cc: syzkaller-bugs, linux-kernel, Peilin Ye, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` equals to 7 or 8. Fix it. This issue has been reported as a KMSAN uninit-value bug, because in such a case, ax25_connect() reaches into the uninitialized portion of the `struct sockaddr_storage` statically allocated in __sys_connect(). It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because `addr_len` is guaranteed to be less than or equal to `sizeof(struct full_sockaddr_ax25)`. Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> --- net/ax25/af_ax25.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index fd91cd34f25e..ef5bf116157a 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, if (addr_len > sizeof(struct sockaddr_ax25) && fsa->fsa_ax25.sax25_ndigis != 0) { /* Valid number of digipeaters ? */ - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { + if (fsa->fsa_ax25.sax25_ndigis < 1 || + addr_len < sizeof(struct sockaddr_ax25) + + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { err = -EINVAL; goto out_release; } -- 2.25.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-22 15:19 ` Peilin Ye @ 2020-07-23 0:57 ` David Miller -1 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2020-07-23 0:57 UTC (permalink / raw) To: yepeilin.cs Cc: jreuter, ralf, gregkh, syzkaller-bugs, linux-kernel-mentees, kuba, linux-hams, netdev, linux-kernel From: Peilin Ye <yepeilin.cs@gmail.com> Date: Wed, 22 Jul 2020 11:19:01 -0400 > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > equals to 7 or 8. Fix it. > > This issue has been reported as a KMSAN uninit-value bug, because in such > a case, ax25_connect() reaches into the uninitialized portion of the > `struct sockaddr_storage` statically allocated in __sys_connect(). > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > `addr_len` is guaranteed to be less than or equal to > `sizeof(struct full_sockaddr_ax25)`. > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> Applied and queued up for -stable, thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 0:57 ` David Miller 0 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2020-07-23 0:57 UTC (permalink / raw) To: yepeilin.cs Cc: syzkaller-bugs, linux-kernel, ralf, netdev, linux-hams, kuba, linux-kernel-mentees, jreuter From: Peilin Ye <yepeilin.cs@gmail.com> Date: Wed, 22 Jul 2020 11:19:01 -0400 > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > equals to 7 or 8. Fix it. > > This issue has been reported as a KMSAN uninit-value bug, because in such > a case, ax25_connect() reaches into the uninitialized portion of the > `struct sockaddr_storage` statically allocated in __sys_connect(). > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > `addr_len` is guaranteed to be less than or equal to > `sizeof(struct full_sockaddr_ax25)`. > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> Applied and queued up for -stable, thanks. _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net] AX.25: Prevent integer overflows in connect and sendmsg 2020-07-23 0:57 ` David Miller @ 2020-07-23 14:49 ` Dan Carpenter -1 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 14:49 UTC (permalink / raw) To: Joerg Reuter, Peilin Ye Cc: Ralf Baechle, David S. Miller, Jakub Kicinski, linux-hams, netdev, gregkh, syzkaller-bugs, linux-kernel-mentees We recently added some bounds checking in ax25_connect() and ax25_sendmsg() and we so we removed the AX25_MAX_DIGIS checks because they were no longer required. Unfortunately, I believe they are required to prevent integer overflows so I have added them back. Fixes: 8885bb0621f0 ("AX.25: Prevent out-of-bounds read in ax25_sendmsg()") Fixes: 2f2a7ffad5c6 ("AX.25: Fix out-of-bounds read in ax25_connect()") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- From code review. Not tested. It should be harmless though. net/ax25/af_ax25.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index 0862fe49d434..dec3f35467c9 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1188,6 +1188,7 @@ static int __must_check ax25_connect(struct socket *sock, fsa->fsa_ax25.sax25_ndigis != 0) { /* Valid number of digipeaters ? */ if (fsa->fsa_ax25.sax25_ndigis < 1 || + fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS || addr_len < sizeof(struct sockaddr_ax25) + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { err = -EINVAL; @@ -1509,7 +1510,9 @@ static int ax25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) struct full_sockaddr_ax25 *fsa = (struct full_sockaddr_ax25 *)usax; /* Valid number of digipeaters ? */ - if (usax->sax25_ndigis < 1 || addr_len < sizeof(struct sockaddr_ax25) + + if (usax->sax25_ndigis < 1 || + usax->sax25_ndigis > AX25_MAX_DIGIS || + addr_len < sizeof(struct sockaddr_ax25) + sizeof(ax25_address) * usax->sax25_ndigis) { err = -EINVAL; goto out; -- 2.27.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Linux-kernel-mentees] [PATCH net] AX.25: Prevent integer overflows in connect and sendmsg @ 2020-07-23 14:49 ` Dan Carpenter 0 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 14:49 UTC (permalink / raw) To: Joerg Reuter, Peilin Ye Cc: linux-hams, netdev, syzkaller-bugs, Ralf Baechle, Jakub Kicinski, linux-kernel-mentees, David S. Miller We recently added some bounds checking in ax25_connect() and ax25_sendmsg() and we so we removed the AX25_MAX_DIGIS checks because they were no longer required. Unfortunately, I believe they are required to prevent integer overflows so I have added them back. Fixes: 8885bb0621f0 ("AX.25: Prevent out-of-bounds read in ax25_sendmsg()") Fixes: 2f2a7ffad5c6 ("AX.25: Fix out-of-bounds read in ax25_connect()") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- From code review. Not tested. It should be harmless though. net/ax25/af_ax25.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index 0862fe49d434..dec3f35467c9 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1188,6 +1188,7 @@ static int __must_check ax25_connect(struct socket *sock, fsa->fsa_ax25.sax25_ndigis != 0) { /* Valid number of digipeaters ? */ if (fsa->fsa_ax25.sax25_ndigis < 1 || + fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS || addr_len < sizeof(struct sockaddr_ax25) + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { err = -EINVAL; @@ -1509,7 +1510,9 @@ static int ax25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) struct full_sockaddr_ax25 *fsa = (struct full_sockaddr_ax25 *)usax; /* Valid number of digipeaters ? */ - if (usax->sax25_ndigis < 1 || addr_len < sizeof(struct sockaddr_ax25) + + if (usax->sax25_ndigis < 1 || + usax->sax25_ndigis > AX25_MAX_DIGIS || + addr_len < sizeof(struct sockaddr_ax25) + sizeof(ax25_address) * usax->sax25_ndigis) { err = -EINVAL; goto out; -- 2.27.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net] AX.25: Prevent integer overflows in connect and sendmsg 2020-07-23 14:49 ` [Linux-kernel-mentees] " Dan Carpenter @ 2020-07-23 19:10 ` David Miller -1 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2020-07-23 19:10 UTC (permalink / raw) To: dan.carpenter Cc: jreuter, yepeilin.cs, ralf, kuba, linux-hams, netdev, gregkh, syzkaller-bugs, linux-kernel-mentees From: Dan Carpenter <dan.carpenter@oracle.com> Date: Thu, 23 Jul 2020 17:49:57 +0300 > We recently added some bounds checking in ax25_connect() and > ax25_sendmsg() and we so we removed the AX25_MAX_DIGIS checks because > they were no longer required. > > Unfortunately, I believe they are required to prevent integer overflows > so I have added them back. > > Fixes: 8885bb0621f0 ("AX.25: Prevent out-of-bounds read in ax25_sendmsg()") > Fixes: 2f2a7ffad5c6 ("AX.25: Fix out-of-bounds read in ax25_connect()") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, thanks Dan. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Prevent integer overflows in connect and sendmsg @ 2020-07-23 19:10 ` David Miller 0 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2020-07-23 19:10 UTC (permalink / raw) To: dan.carpenter Cc: netdev, syzkaller-bugs, ralf, linux-hams, kuba, linux-kernel-mentees, yepeilin.cs, jreuter From: Dan Carpenter <dan.carpenter@oracle.com> Date: Thu, 23 Jul 2020 17:49:57 +0300 > We recently added some bounds checking in ax25_connect() and > ax25_sendmsg() and we so we removed the AX25_MAX_DIGIS checks because > they were no longer required. > > Unfortunately, I believe they are required to prevent integer overflows > so I have added them back. > > Fixes: 8885bb0621f0 ("AX.25: Prevent out-of-bounds read in ax25_sendmsg()") > Fixes: 2f2a7ffad5c6 ("AX.25: Fix out-of-bounds read in ax25_connect()") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, thanks Dan. _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-23 0:57 ` David Miller @ 2020-07-23 15:15 ` Peilin Ye -1 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 15:15 UTC (permalink / raw) To: David Miller Cc: jreuter, ralf, gregkh, syzkaller-bugs, linux-kernel-mentees, kuba, linux-hams, netdev, linux-kernel On Wed, Jul 22, 2020 at 05:57:14PM -0700, David Miller wrote: > From: Peilin Ye <yepeilin.cs@gmail.com> > Date: Wed, 22 Jul 2020 11:19:01 -0400 > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > equals to 7 or 8. Fix it. > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > a case, ax25_connect() reaches into the uninitialized portion of the > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > `addr_len` is guaranteed to be less than or equal to > > `sizeof(struct full_sockaddr_ax25)`. > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > Applied and queued up for -stable, thanks. Thank you for reviewing my patch! Peilin Ye ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 15:15 ` Peilin Ye 0 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 15:15 UTC (permalink / raw) To: David Miller Cc: syzkaller-bugs, linux-kernel, ralf, netdev, linux-hams, kuba, linux-kernel-mentees, jreuter On Wed, Jul 22, 2020 at 05:57:14PM -0700, David Miller wrote: > From: Peilin Ye <yepeilin.cs@gmail.com> > Date: Wed, 22 Jul 2020 11:19:01 -0400 > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > equals to 7 or 8. Fix it. > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > a case, ax25_connect() reaches into the uninitialized portion of the > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > `addr_len` is guaranteed to be less than or equal to > > `sizeof(struct full_sockaddr_ax25)`. > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > Applied and queued up for -stable, thanks. Thank you for reviewing my patch! Peilin Ye _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-22 15:19 ` Peilin Ye @ 2020-07-23 14:28 ` Dan Carpenter -1 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 14:28 UTC (permalink / raw) To: Peilin Ye Cc: Joerg Reuter, Ralf Baechle, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > equals to 7 or 8. Fix it. > > This issue has been reported as a KMSAN uninit-value bug, because in such > a case, ax25_connect() reaches into the uninitialized portion of the > `struct sockaddr_storage` statically allocated in __sys_connect(). > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > `addr_len` is guaranteed to be less than or equal to > `sizeof(struct full_sockaddr_ax25)`. > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > --- > net/ax25/af_ax25.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > index fd91cd34f25e..ef5bf116157a 100644 > --- a/net/ax25/af_ax25.c > +++ b/net/ax25/af_ax25.c > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > if (addr_len > sizeof(struct sockaddr_ax25) && > fsa->fsa_ax25.sax25_ndigis != 0) { > /* Valid number of digipeaters ? */ > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > + addr_len < sizeof(struct sockaddr_ax25) + > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an integer overflow so you still need the "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. regards, dan carpenter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 14:28 ` Dan Carpenter 0 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 14:28 UTC (permalink / raw) To: Peilin Ye Cc: syzkaller-bugs, linux-kernel, Ralf Baechle, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller, Joerg Reuter On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > equals to 7 or 8. Fix it. > > This issue has been reported as a KMSAN uninit-value bug, because in such > a case, ax25_connect() reaches into the uninitialized portion of the > `struct sockaddr_storage` statically allocated in __sys_connect(). > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > `addr_len` is guaranteed to be less than or equal to > `sizeof(struct full_sockaddr_ax25)`. > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > --- > net/ax25/af_ax25.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > index fd91cd34f25e..ef5bf116157a 100644 > --- a/net/ax25/af_ax25.c > +++ b/net/ax25/af_ax25.c > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > if (addr_len > sizeof(struct sockaddr_ax25) && > fsa->fsa_ax25.sax25_ndigis != 0) { > /* Valid number of digipeaters ? */ > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > + addr_len < sizeof(struct sockaddr_ax25) + > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an integer overflow so you still need the "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. regards, dan carpenter _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-23 14:28 ` Dan Carpenter @ 2020-07-23 15:13 ` Peilin Ye -1 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 15:13 UTC (permalink / raw) To: Dan Carpenter Cc: Joerg Reuter, Ralf Baechle, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > equals to 7 or 8. Fix it. > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > a case, ax25_connect() reaches into the uninitialized portion of the > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > `addr_len` is guaranteed to be less than or equal to > > `sizeof(struct full_sockaddr_ax25)`. > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > --- > > net/ax25/af_ax25.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > index fd91cd34f25e..ef5bf116157a 100644 > > --- a/net/ax25/af_ax25.c > > +++ b/net/ax25/af_ax25.c > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > if (addr_len > sizeof(struct sockaddr_ax25) && > > fsa->fsa_ax25.sax25_ndigis != 0) { > > /* Valid number of digipeaters ? */ > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > + addr_len < sizeof(struct sockaddr_ax25) + > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > integer overflow so you still need the > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. Thank you for fixing this up! I did some math but I didn't think of that. Will be more careful when removing things. Peilin Ye ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 15:13 ` Peilin Ye 0 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 15:13 UTC (permalink / raw) To: Dan Carpenter Cc: syzkaller-bugs, linux-kernel, Ralf Baechle, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller, Joerg Reuter On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > equals to 7 or 8. Fix it. > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > a case, ax25_connect() reaches into the uninitialized portion of the > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > `addr_len` is guaranteed to be less than or equal to > > `sizeof(struct full_sockaddr_ax25)`. > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > --- > > net/ax25/af_ax25.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > index fd91cd34f25e..ef5bf116157a 100644 > > --- a/net/ax25/af_ax25.c > > +++ b/net/ax25/af_ax25.c > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > if (addr_len > sizeof(struct sockaddr_ax25) && > > fsa->fsa_ax25.sax25_ndigis != 0) { > > /* Valid number of digipeaters ? */ > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > + addr_len < sizeof(struct sockaddr_ax25) + > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > integer overflow so you still need the > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. Thank you for fixing this up! I did some math but I didn't think of that. Will be more careful when removing things. Peilin Ye _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-23 15:13 ` Peilin Ye @ 2020-07-23 15:50 ` Dan Carpenter -1 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 15:50 UTC (permalink / raw) To: Peilin Ye Cc: Joerg Reuter, Ralf Baechle, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: > On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > > equals to 7 or 8. Fix it. > > > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > > a case, ax25_connect() reaches into the uninitialized portion of the > > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > > `addr_len` is guaranteed to be less than or equal to > > > `sizeof(struct full_sockaddr_ax25)`. > > > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > > --- > > > net/ax25/af_ax25.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > > index fd91cd34f25e..ef5bf116157a 100644 > > > --- a/net/ax25/af_ax25.c > > > +++ b/net/ax25/af_ax25.c > > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > > if (addr_len > sizeof(struct sockaddr_ax25) && > > > fsa->fsa_ax25.sax25_ndigis != 0) { > > > /* Valid number of digipeaters ? */ > > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > > + addr_len < sizeof(struct sockaddr_ax25) + > > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > > integer overflow so you still need the > > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. > > Thank you for fixing this up! I did some math but I didn't think of > that. Will be more careful when removing things. No problem. You had the right approach to look for ways to clean things up. Your patches make me happy because you're trying to fix important bugs. regards, dan carpenter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 15:50 ` Dan Carpenter 0 siblings, 0 replies; 20+ messages in thread From: Dan Carpenter @ 2020-07-23 15:50 UTC (permalink / raw) To: Peilin Ye Cc: syzkaller-bugs, linux-kernel, Ralf Baechle, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller, Joerg Reuter On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: > On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > > equals to 7 or 8. Fix it. > > > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > > a case, ax25_connect() reaches into the uninitialized portion of the > > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > > `addr_len` is guaranteed to be less than or equal to > > > `sizeof(struct full_sockaddr_ax25)`. > > > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > > --- > > > net/ax25/af_ax25.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > > index fd91cd34f25e..ef5bf116157a 100644 > > > --- a/net/ax25/af_ax25.c > > > +++ b/net/ax25/af_ax25.c > > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > > if (addr_len > sizeof(struct sockaddr_ax25) && > > > fsa->fsa_ax25.sax25_ndigis != 0) { > > > /* Valid number of digipeaters ? */ > > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > > + addr_len < sizeof(struct sockaddr_ax25) + > > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > > integer overflow so you still need the > > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. > > Thank you for fixing this up! I did some math but I didn't think of > that. Will be more careful when removing things. No problem. You had the right approach to look for ways to clean things up. Your patches make me happy because you're trying to fix important bugs. regards, dan carpenter _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-23 15:50 ` Dan Carpenter @ 2020-07-23 16:43 ` Peilin Ye -1 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 16:43 UTC (permalink / raw) To: Dan Carpenter Cc: Joerg Reuter, Ralf Baechle, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel On Thu, Jul 23, 2020 at 06:50:58PM +0300, Dan Carpenter wrote: > On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: > > On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > > > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > > > equals to 7 or 8. Fix it. > > > > > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > > > a case, ax25_connect() reaches into the uninitialized portion of the > > > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > > > `addr_len` is guaranteed to be less than or equal to > > > > `sizeof(struct full_sockaddr_ax25)`. > > > > > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > > > --- > > > > net/ax25/af_ax25.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > > > index fd91cd34f25e..ef5bf116157a 100644 > > > > --- a/net/ax25/af_ax25.c > > > > +++ b/net/ax25/af_ax25.c > > > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > > > if (addr_len > sizeof(struct sockaddr_ax25) && > > > > fsa->fsa_ax25.sax25_ndigis != 0) { > > > > /* Valid number of digipeaters ? */ > > > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > > > + addr_len < sizeof(struct sockaddr_ax25) + > > > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > > > > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > > > integer overflow so you still need the > > > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. > > > > Thank you for fixing this up! I did some math but I didn't think of > > that. Will be more careful when removing things. > > No problem. You had the right approach to look for ways to clean things > up. > > Your patches make me happy because you're trying to fix important bugs. It is very encouraging to hear that! I will try to do what I can do. Thank you, Peilin Ye ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 16:43 ` Peilin Ye 0 siblings, 0 replies; 20+ messages in thread From: Peilin Ye @ 2020-07-23 16:43 UTC (permalink / raw) To: Dan Carpenter Cc: syzkaller-bugs, linux-kernel, Ralf Baechle, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller, Joerg Reuter On Thu, Jul 23, 2020 at 06:50:58PM +0300, Dan Carpenter wrote: > On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: > > On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: > > > On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: > > > > Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. > > > > ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` > > > > equals to 7 or 8. Fix it. > > > > > > > > This issue has been reported as a KMSAN uninit-value bug, because in such > > > > a case, ax25_connect() reaches into the uninitialized portion of the > > > > `struct sockaddr_storage` statically allocated in __sys_connect(). > > > > > > > > It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because > > > > `addr_len` is guaranteed to be less than or equal to > > > > `sizeof(struct full_sockaddr_ax25)`. > > > > > > > > Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com > > > > Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 > > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> > > > > --- > > > > net/ax25/af_ax25.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c > > > > index fd91cd34f25e..ef5bf116157a 100644 > > > > --- a/net/ax25/af_ax25.c > > > > +++ b/net/ax25/af_ax25.c > > > > @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, > > > > if (addr_len > sizeof(struct sockaddr_ax25) && > > > > fsa->fsa_ax25.sax25_ndigis != 0) { > > > > /* Valid number of digipeaters ? */ > > > > - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { > > > > + if (fsa->fsa_ax25.sax25_ndigis < 1 || > > > > + addr_len < sizeof(struct sockaddr_ax25) + > > > > + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { > > > > > > The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an > > > integer overflow so you still need the > > > "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. > > > > Thank you for fixing this up! I did some math but I didn't think of > > that. Will be more careful when removing things. > > No problem. You had the right approach to look for ways to clean things > up. > > Your patches make me happy because you're trying to fix important bugs. It is very encouraging to hear that! I will try to do what I can do. Thank you, Peilin Ye _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() 2020-07-23 15:50 ` Dan Carpenter @ 2020-07-23 21:41 ` vk2tv -1 siblings, 0 replies; 20+ messages in thread From: vk2tv @ 2020-07-23 21:41 UTC (permalink / raw) To: Dan Carpenter, Peilin Ye Cc: Joerg Reuter, Ralf Baechle, Greg Kroah-Hartman, syzkaller-bugs, linux-kernel-mentees, David S . Miller, Jakub Kicinski, linux-hams, netdev, linux-kernel On 24/7/20 1:50 am, Dan Carpenter wrote: > On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: >> On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: >>> On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: >>>> Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. >>>> ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` >>>> equals to 7 or 8. Fix it. >>>> >>>> This issue has been reported as a KMSAN uninit-value bug, because in such >>>> a case, ax25_connect() reaches into the uninitialized portion of the >>>> `struct sockaddr_storage` statically allocated in __sys_connect(). >>>> >>>> It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because >>>> `addr_len` is guaranteed to be less than or equal to >>>> `sizeof(struct full_sockaddr_ax25)`. >>>> >>>> Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com >>>> Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 >>>> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> >>>> --- >>>> net/ax25/af_ax25.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c >>>> index fd91cd34f25e..ef5bf116157a 100644 >>>> --- a/net/ax25/af_ax25.c >>>> +++ b/net/ax25/af_ax25.c >>>> @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, >>>> if (addr_len > sizeof(struct sockaddr_ax25) && >>>> fsa->fsa_ax25.sax25_ndigis != 0) { >>>> /* Valid number of digipeaters ? */ >>>> - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { >>>> + if (fsa->fsa_ax25.sax25_ndigis < 1 || >>>> + addr_len < sizeof(struct sockaddr_ax25) + >>>> + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { >>> The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an >>> integer overflow so you still need the >>> "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. >> Thank you for fixing this up! I did some math but I didn't think of >> that. Will be more careful when removing things. > No problem. You had the right approach to look for ways to clean things > up. > > Your patches make me happy because you're trying to fix important bugs. > > regards, > dan carpenter As a long-term user (25 years) of kernel ax25 I appreciate any and all efforts to improve the code (which I mostly don't understand), and I applaud those individuals rising to the task. Thanks guys (and gals). Ray vk2tv ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() @ 2020-07-23 21:41 ` vk2tv 0 siblings, 0 replies; 20+ messages in thread From: vk2tv @ 2020-07-23 21:41 UTC (permalink / raw) To: Dan Carpenter, Peilin Ye Cc: syzkaller-bugs, linux-kernel, Ralf Baechle, netdev, linux-hams, Jakub Kicinski, linux-kernel-mentees, David S . Miller, Joerg Reuter On 24/7/20 1:50 am, Dan Carpenter wrote: > On Thu, Jul 23, 2020 at 11:13:55AM -0400, Peilin Ye wrote: >> On Thu, Jul 23, 2020 at 05:28:15PM +0300, Dan Carpenter wrote: >>> On Wed, Jul 22, 2020 at 11:19:01AM -0400, Peilin Ye wrote: >>>> Checks on `addr_len` and `fsa->fsa_ax25.sax25_ndigis` are insufficient. >>>> ax25_connect() can go out of bounds when `fsa->fsa_ax25.sax25_ndigis` >>>> equals to 7 or 8. Fix it. >>>> >>>> This issue has been reported as a KMSAN uninit-value bug, because in such >>>> a case, ax25_connect() reaches into the uninitialized portion of the >>>> `struct sockaddr_storage` statically allocated in __sys_connect(). >>>> >>>> It is safe to remove `fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS` because >>>> `addr_len` is guaranteed to be less than or equal to >>>> `sizeof(struct full_sockaddr_ax25)`. >>>> >>>> Reported-by: syzbot+c82752228ed975b0a623@syzkaller.appspotmail.com >>>> Link: https://syzkaller.appspot.com/bug?id=55ef9d629f3b3d7d70b69558015b63b48d01af66 >>>> Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com> >>>> --- >>>> net/ax25/af_ax25.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c >>>> index fd91cd34f25e..ef5bf116157a 100644 >>>> --- a/net/ax25/af_ax25.c >>>> +++ b/net/ax25/af_ax25.c >>>> @@ -1187,7 +1187,9 @@ static int __must_check ax25_connect(struct socket *sock, >>>> if (addr_len > sizeof(struct sockaddr_ax25) && >>>> fsa->fsa_ax25.sax25_ndigis != 0) { >>>> /* Valid number of digipeaters ? */ >>>> - if (fsa->fsa_ax25.sax25_ndigis < 1 || fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS) { >>>> + if (fsa->fsa_ax25.sax25_ndigis < 1 || >>>> + addr_len < sizeof(struct sockaddr_ax25) + >>>> + sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) { >>> The "sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis" can have an >>> integer overflow so you still need the >>> "fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS" check. >> Thank you for fixing this up! I did some math but I didn't think of >> that. Will be more careful when removing things. > No problem. You had the right approach to look for ways to clean things > up. > > Your patches make me happy because you're trying to fix important bugs. > > regards, > dan carpenter As a long-term user (25 years) of kernel ax25 I appreciate any and all efforts to improve the code (which I mostly don't understand), and I applaud those individuals rising to the task. Thanks guys (and gals). Ray vk2tv _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-07-23 21:56 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-22 15:19 [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() Peilin Ye 2020-07-22 15:19 ` Peilin Ye 2020-07-23 0:57 ` David Miller 2020-07-23 0:57 ` David Miller 2020-07-23 14:49 ` [PATCH net] AX.25: Prevent integer overflows in connect and sendmsg Dan Carpenter 2020-07-23 14:49 ` [Linux-kernel-mentees] " Dan Carpenter 2020-07-23 19:10 ` David Miller 2020-07-23 19:10 ` [Linux-kernel-mentees] " David Miller 2020-07-23 15:15 ` [Linux-kernel-mentees] [PATCH net] AX.25: Fix out-of-bounds read in ax25_connect() Peilin Ye 2020-07-23 15:15 ` Peilin Ye 2020-07-23 14:28 ` Dan Carpenter 2020-07-23 14:28 ` Dan Carpenter 2020-07-23 15:13 ` Peilin Ye 2020-07-23 15:13 ` Peilin Ye 2020-07-23 15:50 ` Dan Carpenter 2020-07-23 15:50 ` Dan Carpenter 2020-07-23 16:43 ` Peilin Ye 2020-07-23 16:43 ` Peilin Ye 2020-07-23 21:41 ` vk2tv 2020-07-23 21:41 ` vk2tv
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.