| Message ID | 20241111-sockptr-copy-ret-fix-v1-1-a520083a93fb@rbox.co (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eb94b7bb10109a14a5431a67e5d8e31cfa06b395 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: Make copy_safe_from_sockptr() match documentation | expand |
On Mon, 11 Nov 2024 00:17:34 +0100 Michal Luczaj wrote: > copy_safe_from_sockptr() > return copy_from_sockptr() > return copy_from_sockptr_offset() > return copy_from_user() > > copy_from_user() does not return an error on fault. Instead, it returns a > number of bytes that were not copied. Have it handled. > > Fixes: 6309863b31dd ("net: add copy_safe_from_sockptr() helper") > Signed-off-by: Michal Luczaj <mhal@rbox.co> > --- > Patch has a side effect: it un-breaks garbage input handling of > nfc_llcp_setsockopt() and mISDN's data_sock_setsockopt(). I'll move this to the commit message, it's important. Are you planning to scan callers of copy_from_sockptr() ? I looked at 3 callers who save the return value, 2 of them are buggy :S
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 11 Nov 2024 00:17:34 +0100 you wrote: > copy_safe_from_sockptr() > return copy_from_sockptr() > return copy_from_sockptr_offset() > return copy_from_user() > > copy_from_user() does not return an error on fault. Instead, it returns a > number of bytes that were not copied. Have it handled. > > [...] Here is the summary with links: - [net] net: Make copy_safe_from_sockptr() match documentation https://git.kernel.org/netdev/net/c/eb94b7bb1010 You are awesome, thank you!
On 11/14/24 04:29, Jakub Kicinski wrote: > On Mon, 11 Nov 2024 00:17:34 +0100 Michal Luczaj wrote: >> copy_safe_from_sockptr() >> return copy_from_sockptr() >> return copy_from_sockptr_offset() >> return copy_from_user() >> >> copy_from_user() does not return an error on fault. Instead, it returns a >> number of bytes that were not copied. Have it handled. >> >> Fixes: 6309863b31dd ("net: add copy_safe_from_sockptr() helper") >> Signed-off-by: Michal Luczaj <mhal@rbox.co> >> --- >> Patch has a side effect: it un-breaks garbage input handling of >> nfc_llcp_setsockopt() and mISDN's data_sock_setsockopt(). > > I'll move this to the commit message, it's important. > > Are you planning to scan callers of copy_from_sockptr() ? > I looked at 3 callers who save the return value, 2 of them are buggy :S Sure, I took a look: https://lore.kernel.org/netdev/20241115-sockptr-copy-fixes-v1-0-d183c87fcbd5@rbox.co/ Have I missed anything? Thanks, Michal
On Fri, 15 Nov 2024 00:36:35 +0100 Michal Luczaj wrote: > > I'll move this to the commit message, it's important. > > > > Are you planning to scan callers of copy_from_sockptr() ? > > I looked at 3 callers who save the return value, 2 of them are buggy :S > > Sure, I took a look: > https://lore.kernel.org/netdev/20241115-sockptr-copy-fixes-v1-0-d183c87fcbd5@rbox.co/ > Have I missed anything? Looks like you covered the two I spotted - rxrpc and llc, thanks!
diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h index fc5a206c40435fca5bc97e9e44f47277ac2aa04c..195debe2b1dbc5abf768aa806eb6c73b99421e27 100644 --- a/include/linux/sockptr.h +++ b/include/linux/sockptr.h @@ -77,7 +77,9 @@ static inline int copy_safe_from_sockptr(void *dst, size_t ksize, { if (optlen < ksize) return -EINVAL; - return copy_from_sockptr(dst, optval, ksize); + if (copy_from_sockptr(dst, optval, ksize)) + return -EFAULT; + return 0; } static inline int copy_struct_from_sockptr(void *dst, size_t ksize,
copy_safe_from_sockptr() return copy_from_sockptr() return copy_from_sockptr_offset() return copy_from_user() copy_from_user() does not return an error on fault. Instead, it returns a number of bytes that were not copied. Have it handled. Fixes: 6309863b31dd ("net: add copy_safe_from_sockptr() helper") Signed-off-by: Michal Luczaj <mhal@rbox.co> --- Patch has a side effect: it un-breaks garbage input handling of nfc_llcp_setsockopt() and mISDN's data_sock_setsockopt(). --- include/linux/sockptr.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) --- base-commit: 252e01e68241d33bfe0ed1fc333220d9bd8b06df change-id: 20241109-sockptr-copy-ret-fix-ca6f41413d93 Best regards,