| Message ID | 1520856964-132516-1-git-send-email-weiyongjun1@huawei.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] sctp: fix error return code in sctp_sendmsg_new_asoc() | expand |
On Mon, Mar 12, 2018 at 8:16 PM, Wei Yongjun <weiyongjun1@huawei.com> wrote: > Return error code -EINVAL in the address len check error handling > case since 'err' can be overwrite to 0 by 'err = sctp_verify_addr()' > in the for loop. > > Fixes: 2c0dbaa0c43d ("sctp: add support for SCTP_DSTADDRV4/6 Information for sendmsg") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > net/sctp/socket.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 7d3476a..5e7bfc2 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1761,16 +1761,20 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, > memset(daddr, 0, sizeof(*daddr)); > dlen = cmsg->cmsg_len - sizeof(struct cmsghdr); > if (cmsg->cmsg_type == SCTP_DSTADDRV4) { > - if (dlen < sizeof(struct in_addr)) > + if (dlen < sizeof(struct in_addr)) { > + err = -EINVAL; > goto free; > + } > > dlen = sizeof(struct in_addr); > daddr->v4.sin_family = AF_INET; > daddr->v4.sin_port = htons(asoc->peer.port); > memcpy(&daddr->v4.sin_addr, CMSG_DATA(cmsg), dlen); > } else { > - if (dlen < sizeof(struct in6_addr)) > + if (dlen < sizeof(struct in6_addr)) { > + err = -EINVAL; > goto free; > + } > > dlen = sizeof(struct in6_addr); > daddr->v6.sin6_family = AF_INET6; > Unfortunately, this can't be fixed right now, because of https://lkml.org/lkml/2018/3/7/1175 Pls hold it until net-next merges linus tree. Btw, the 'err' initialization can also be removed in your patch. Thanks.
From: Wei Yongjun <weiyongjun1@huawei.com> Date: Mon, 12 Mar 2018 12:16:04 +0000 > Return error code -EINVAL in the address len check error handling > case since 'err' can be overwrite to 0 by 'err = sctp_verify_addr()' > in the for loop. > > Fixes: 2c0dbaa0c43d ("sctp: add support for SCTP_DSTADDRV4/6 Information for sendmsg") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Xin, I do not want to have a bug fix like this blocked because of that SCTP options merge nightmare which is of no fault of our own.
On Mon, Mar 12, 2018 at 12:16:04PM +0000, Wei Yongjun wrote: > Return error code -EINVAL in the address len check error handling > case since 'err' can be overwrite to 0 by 'err = sctp_verify_addr()' > in the for loop. > > Fixes: 2c0dbaa0c43d ("sctp: add support for SCTP_DSTADDRV4/6 Information for sendmsg") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > net/sctp/socket.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 7d3476a..5e7bfc2 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -1761,16 +1761,20 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, > memset(daddr, 0, sizeof(*daddr)); > dlen = cmsg->cmsg_len - sizeof(struct cmsghdr); > if (cmsg->cmsg_type == SCTP_DSTADDRV4) { > - if (dlen < sizeof(struct in_addr)) > + if (dlen < sizeof(struct in_addr)) { > + err = -EINVAL; > goto free; > + } > > dlen = sizeof(struct in_addr); > daddr->v4.sin_family = AF_INET; > daddr->v4.sin_port = htons(asoc->peer.port); > memcpy(&daddr->v4.sin_addr, CMSG_DATA(cmsg), dlen); > } else { > - if (dlen < sizeof(struct in6_addr)) > + if (dlen < sizeof(struct in6_addr)) { > + err = -EINVAL; > goto free; > + } > > dlen = sizeof(struct in6_addr); > daddr->v6.sin6_family = AF_INET6; > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
On Mon, Mar 12, 2018 at 09:38:53PM +0800, Xin Long wrote: > On Mon, Mar 12, 2018 at 8:16 PM, Wei Yongjun <weiyongjun1@huawei.com> wrote: ... > Btw, the 'err' initialization can also be removed in your patch. I second this.
On Mon, Mar 12, 2018 at 11:19 PM, David Miller <davem@davemloft.net> wrote: > From: Wei Yongjun <weiyongjun1@huawei.com> > Date: Mon, 12 Mar 2018 12:16:04 +0000 > >> Return error code -EINVAL in the address len check error handling >> case since 'err' can be overwrite to 0 by 'err = sctp_verify_addr()' >> in the for loop. >> >> Fixes: 2c0dbaa0c43d ("sctp: add support for SCTP_DSTADDRV4/6 Information for sendmsg") >> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > > Xin, I do not want to have a bug fix like this blocked because of that > SCTP options merge nightmare which is of no fault of our own. Okay, sorry, I was over worried recently, thank you.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 7d3476a..5e7bfc2 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1761,16 +1761,20 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags, memset(daddr, 0, sizeof(*daddr)); dlen = cmsg->cmsg_len - sizeof(struct cmsghdr); if (cmsg->cmsg_type == SCTP_DSTADDRV4) { - if (dlen < sizeof(struct in_addr)) + if (dlen < sizeof(struct in_addr)) { + err = -EINVAL; goto free; + } dlen = sizeof(struct in_addr); daddr->v4.sin_family = AF_INET; daddr->v4.sin_port = htons(asoc->peer.port); memcpy(&daddr->v4.sin_addr, CMSG_DATA(cmsg), dlen); } else { - if (dlen < sizeof(struct in6_addr)) + if (dlen < sizeof(struct in6_addr)) { + err = -EINVAL; goto free; + } dlen = sizeof(struct in6_addr); daddr->v6.sin6_family = AF_INET6;
Return error code -EINVAL in the address len check error handling case since 'err' can be overwrite to 0 by 'err = sctp_verify_addr()' in the for loop. Fixes: 2c0dbaa0c43d ("sctp: add support for SCTP_DSTADDRV4/6 Information for sendmsg") Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- net/sctp/socket.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)