| Message ID | 20241115-sockptr-copy-fixes-v2-2-9b1254c18b7a@rbox.co (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Fix some callers of copy_from_sockptr() | expand |
On 2024-11-15 05:21, Michal Luczaj wrote: > copy_from_sockptr()'s non-zero result represents the number of bytes that > could not be copied. Turn that into EFAULT. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Michal Luczaj <mhal@rbox.co> > --- > net/llc/af_llc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c > index 4eb52add7103b0f83d6fe7318abf1d1af533d254..711c8a7a423f1cf1b03e684a6e23c8eefbab830f 100644 > --- a/net/llc/af_llc.c > +++ b/net/llc/af_llc.c > @@ -1096,12 +1096,12 @@ static int llc_ui_setsockopt(struct socket *sock, int level, int optname, > int rc = -EINVAL; > > lock_sock(sk); > - if (unlikely(level != SOL_LLC || optlen != sizeof(int))) > + if (unlikely(level != SOL_LLC || optlen != sizeof(opt))) > goto out; > - rc = copy_from_sockptr(&opt, optval, sizeof(opt)); > - if (rc) > + if (copy_from_sockptr(&opt, optval, sizeof(opt))) { > + rc = -EFAULT; > goto out; > - rc = -EINVAL; > + } > switch (optname) { > case LLC_OPT_RETRY: > if (opt > LLC_OPT_MAX_RETRY) > Can copy_from_sockptr() be deprecated here in favour of copy_safe_from_sockptr()?
On 11/16/24 01:59, David Wei wrote: > On 2024-11-15 05:21, Michal Luczaj wrote: >> copy_from_sockptr()'s non-zero result represents the number of bytes that >> could not be copied. Turn that into EFAULT. >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Michal Luczaj <mhal@rbox.co> >> --- >> net/llc/af_llc.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c >> index 4eb52add7103b0f83d6fe7318abf1d1af533d254..711c8a7a423f1cf1b03e684a6e23c8eefbab830f 100644 >> --- a/net/llc/af_llc.c >> +++ b/net/llc/af_llc.c >> @@ -1096,12 +1096,12 @@ static int llc_ui_setsockopt(struct socket *sock, int level, int optname, >> int rc = -EINVAL; >> >> lock_sock(sk); >> - if (unlikely(level != SOL_LLC || optlen != sizeof(int))) >> + if (unlikely(level != SOL_LLC || optlen != sizeof(opt))) >> goto out; >> - rc = copy_from_sockptr(&opt, optval, sizeof(opt)); >> - if (rc) >> + if (copy_from_sockptr(&opt, optval, sizeof(opt))) { >> + rc = -EFAULT; >> goto out; >> - rc = -EINVAL; >> + } >> switch (optname) { >> case LLC_OPT_RETRY: >> if (opt > LLC_OPT_MAX_RETRY) >> > > Can copy_from_sockptr() be deprecated here in favour of > copy_safe_from_sockptr()? Yeah, good point. I'll wait a bit and send v3. Thanks!
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c index 4eb52add7103b0f83d6fe7318abf1d1af533d254..711c8a7a423f1cf1b03e684a6e23c8eefbab830f 100644 --- a/net/llc/af_llc.c +++ b/net/llc/af_llc.c @@ -1096,12 +1096,12 @@ static int llc_ui_setsockopt(struct socket *sock, int level, int optname, int rc = -EINVAL; lock_sock(sk); - if (unlikely(level != SOL_LLC || optlen != sizeof(int))) + if (unlikely(level != SOL_LLC || optlen != sizeof(opt))) goto out; - rc = copy_from_sockptr(&opt, optval, sizeof(opt)); - if (rc) + if (copy_from_sockptr(&opt, optval, sizeof(opt))) { + rc = -EFAULT; goto out; - rc = -EINVAL; + } switch (optname) { case LLC_OPT_RETRY: if (opt > LLC_OPT_MAX_RETRY)
copy_from_sockptr()'s non-zero result represents the number of bytes that could not be copied. Turn that into EFAULT. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Michal Luczaj <mhal@rbox.co> --- net/llc/af_llc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)