[PATCH] include/linux: avoid narrowing length parameter values

[PATCH] include/linux: avoid narrowing length parameter values

[Louis Langholtz]

memcpy_from_msg() and memcpy_to_msg() functions previously called
memcpy_fromiovec() and memcpy_toiovec() functions respectively. The
memcpy_fromiovec() and memcpy_toiovec() functions took a length parameter
of type int. memcpy_from_msg() and memcpy_to_msg() now call
copy_from_iter() and copy_to_iter() functions respectively which take a length
parameter of type size_t. Most code calling the memcpy_from_msg() and
memcpy_to_msg() functions currently pass a length value of type size_t.
This patch updates the memcpy_from_msg() and memcpy_to_msg() functions
concordantly to take the length parameter of type size_t. This also avoids a potential
for data narrowing.

Signed-off-by: Louis Langholtz <lou_langholtz@me.com>
--

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 45e0aa6..ee590fb 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2708,12 +2708,12 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len);
 int skb_vlan_pop(struct sk_buff *skb);
 int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
 
-static inline int memcpy_from_msg(void *data, struct msghdr *msg, int len)
+static inline int memcpy_from_msg(void *data, struct msghdr *msg, size_t len)
 {
        return copy_from_iter(data, len, &msg->msg_iter) == len ? 0 : -EFAULT;
 }
 
-static inline int memcpy_to_msg(struct msghdr *msg, void *data, int len)
+static inline int memcpy_to_msg(struct msghdr *msg, void *data, size_t len)
 {
        return copy_to_iter(data, len, &msg->msg_iter) == len ? 0 : -EFAULT;
 }

Re: [PATCH] include/linux: avoid narrowing length parameter values

[Al Viro]

On Mon, May 18, 2015 at 09:33:10AM -0600, Louis Langholtz wrote:
> memcpy_from_msg() and memcpy_to_msg() functions previously called
> memcpy_fromiovec() and memcpy_toiovec() functions respectively. The
> memcpy_fromiovec() and memcpy_toiovec() functions took a length parameter
> of type int. memcpy_from_msg() and memcpy_to_msg() now call
> copy_from_iter() and copy_to_iter() functions respectively which take a length
> parameter of type size_t. Most code calling the memcpy_from_msg() and
> memcpy_to_msg() functions currently pass a length value of type size_t.
> This patch updates the memcpy_from_msg() and memcpy_to_msg() functions
> concordantly to take the length parameter of type size_t. This also avoids a potential
> for data narrowing.

iov_iter for sendmsg or recvmsg *can't* have more than 2Gb of data; if it
ever does, it's a serious bug.

IOW, NAK - that's pointless.

Re: [PATCH] include/linux: avoid narrowing length parameter values

[Louis Langholtz]

On May 18, 2015, at 9:56 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:

> On Mon, May 18, 2015 at 09:33:10AM -0600, Louis Langholtz wrote:
>> memcpy_from_msg() and memcpy_to_msg() functions previously called
>> memcpy_fromiovec() and memcpy_toiovec() functions respectively. The
>> memcpy_fromiovec() and memcpy_toiovec() functions took a length parameter
>> of type int. memcpy_from_msg() and memcpy_to_msg() now call
>> copy_from_iter() and copy_to_iter() functions respectively which take a length
>> parameter of type size_t. Most code calling the memcpy_from_msg() and
>> memcpy_to_msg() functions currently pass a length value of type size_t.
>> This patch updates the memcpy_from_msg() and memcpy_to_msg() functions
>> concordantly to take the length parameter of type size_t. This also avoids a potential
>> for data narrowing.
> 
> iov_iter for sendmsg or recvmsg *can't* have more than 2Gb of data; if it
> ever does, it's a serious bug.
> 
> IOW, NAK - that's pointless.

I understand that operationally the change is a no-op given the 2Gb limit you
point out. I still don't understand how using size_t instead of int is pointless
however. The change still increases consistency and adds semantically by
using the type (size_t) established for holding the size of an object.

If the position is that weak-typing is better, I can understand that; I just disagree
then. If the position is that u32 would be better (than int because it more closely
matches the 2Gb design limit presuming that the value also can't ever be
negative), I'd also understand not applying this patch and would agree with that
argument (although I'd be bothered then that so much of the relevant code is
already using size_t).