diff mbox

iov_iter: don't revert if csum error

Message ID 080dafec-c57f-0546-4d2f-ce4a31222e25@huawei.com
State New
Headers show

Commit Message

Ding Tianhong April 28, 2017, 12:48 p.m. UTC
The patch 3278682 (make skb_copy_datagram_msg() et.al. preserve
->msg_iter on error) will revert the iov buffer if copy to iter
failed, but it looks no need to revert for csum error, so fix it.

Fixes: 3278682 ("make skb_copy_datagram_msg() et.al. preserve->msg_iter on error")
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

---
 net/core/datagram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.8.3.1

Comments

David Laight April 28, 2017, 1:51 p.m. UTC | #1
From: Sabrina Dubroca

> Sent: 28 April 2017 14:17

...
> >  		if (__skb_checksum_complete(skb))

> > -			goto csum_error;

> > +			goto fault;

> 

> With this patch, skb_copy_and_csum_datagram_msg() will return -EFAULT

> for an incorrect checksum, that doesn't seem right.


Especially since (IIRC) -EFAULT generates SIGSEGV.

	David
Ding Tianhong April 29, 2017, 2:12 a.m. UTC | #2
On 2017/4/28 21:16, Sabrina Dubroca wrote:
> 2017-04-28, 20:48:45 +0800, Ding Tianhong wrote:

>> The patch 3278682 (make skb_copy_datagram_msg() et.al. preserve

>> ->msg_iter on error) will revert the iov buffer if copy to iter

>> failed, but it looks no need to revert for csum error, so fix it.

>>

>> Fixes: 3278682 ("make skb_copy_datagram_msg() et.al. preserve->msg_iter on error")

> 

> Please use 12 digits, ie 327868212381.

> 

>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

>> ---

>>  net/core/datagram.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/net/core/datagram.c b/net/core/datagram.c

>> index f4947e7..475a8e9 100644

>> --- a/net/core/datagram.c

>> +++ b/net/core/datagram.c

>> @@ -760,7 +760,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,

>>

>>  	if (msg_data_left(msg) < chunk) {

>>  		if (__skb_checksum_complete(skb))

>> -			goto csum_error;

>> +			goto fault;

> 

> With this patch, skb_copy_and_csum_datagram_msg() will return -EFAULT

> for an incorrect checksum, that doesn't seem right.

> 


Yes, should not change the return value, thanks.

Ding

>>  		if (skb_copy_datagram_msg(skb, hlen, msg, chunk))

>>  			goto fault;

>>  	} else {

>> -- 

>> 1.8.3.1

>>

>
diff mbox

Patch

diff --git a/net/core/datagram.c b/net/core/datagram.c
index f4947e7..475a8e9 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -760,7 +760,7 @@  int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,

 	if (msg_data_left(msg) < chunk) {
 		if (__skb_checksum_complete(skb))
-			goto csum_error;
+			goto fault;
 		if (skb_copy_datagram_msg(skb, hlen, msg, chunk))
 			goto fault;
 	} else {