netdev - RE: [PATCH v2 net 1/1] tipc: fix kernel warning when sending SYN message

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB9PR05MB907893AE1AECD3CA1F91D40F88D99@DB9PR05MB9078.eurprd05.prod.outlook.com>
Date:   Thu, 9 Feb 2023 11:10:16 +0000
From:   Tung Quang Nguyen <tung.q.nguyen@...tech.com.au>
To:     Paolo Abeni <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "jmaloy@...hat.com" <jmaloy@...hat.com>,
        "ying.xue@...driver.com" <ying.xue@...driver.com>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "syzbot+d43608d061e8847ec9f3@...kaller.appspotmail.com" 
        <syzbot+d43608d061e8847ec9f3@...kaller.appspotmail.com>
Subject: RE: [PATCH v2 net 1/1] tipc: fix kernel warning when sending SYN
 message



>-----Original Message-----
>From: Paolo Abeni <pabeni@...hat.com>
>Sent: Thursday, February 9, 2023 5:39 PM
>To: Tung Quang Nguyen <tung.q.nguyen@...tech.com.au>; netdev@...r.kernel.org
>Cc: davem@...emloft.net; kuba@...nel.org; edumazet@...gle.com; jmaloy@...hat.com; ying.xue@...driver.com;
>viro@...iv.linux.org.uk; syzbot+d43608d061e8847ec9f3@...kaller.appspotmail.com
>Subject: Re: [PATCH v2 net 1/1] tipc: fix kernel warning when sending SYN message
>
>On Wed, 2023-02-08 at 07:07 +0000, Tung Nguyen wrote:
>> When sending a SYN message, this kernel stack trace is observed:
>>
>> ...
>> [   13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
>> ...
>> [   13.398494] Call Trace:
>> [   13.398630]  <TASK>
>> [   13.398630]  ? __alloc_skb+0xed/0x1a0
>> [   13.398630]  tipc_msg_build+0x12c/0x670 [tipc]
>> [   13.398630]  ? shmem_add_to_page_cache.isra.71+0x151/0x290
>> [   13.398630]  __tipc_sendmsg+0x2d1/0x710 [tipc]
>> [   13.398630]  ? tipc_connect+0x1d9/0x230 [tipc]
>> [   13.398630]  ? __local_bh_enable_ip+0x37/0x80
>> [   13.398630]  tipc_connect+0x1d9/0x230 [tipc]
>> [   13.398630]  ? __sys_connect+0x9f/0xd0
>> [   13.398630]  __sys_connect+0x9f/0xd0
>> [   13.398630]  ? preempt_count_add+0x4d/0xa0
>> [   13.398630]  ? fpregs_assert_state_consistent+0x22/0x50
>> [   13.398630]  __x64_sys_connect+0x16/0x20
>> [   13.398630]  do_syscall_64+0x42/0x90
>> [   13.398630]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>
>> It is because commit a41dad905e5a ("iov_iter: saner checks for attempt
>> to copy to/from iterator") has introduced sanity check for copying
>> from/to iov iterator. Lacking of copy direction from the iterator
>> viewpoint would lead to kernel stack trace like above.
>>
>> This commit fixes this issue by initializing the iov iterator with
>> the correct copy direction.
>>
>> Fixes: f25dcc7687d4 ("tipc: tipc ->sendmsg() conversion")
>> Reported-by: syzbot+d43608d061e8847ec9f3@...kaller.appspotmail.com
>> Acked-by: Jon Maloy <jmaloy@...hat.com>
>> Signed-off-by: Tung Nguyen <tung.q.nguyen@...tech.com.au>
>> ---
>> v2: add Fixes tag
>>
>>  net/tipc/msg.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
>> index 5c9fd4791c4b..cce118fea07a 100644
>> --- a/net/tipc/msg.c
>> +++ b/net/tipc/msg.c
>> @@ -381,6 +381,9 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
>>
>>  	msg_set_size(mhdr, msz);
>>
>> +	if (!dsz)
>> +		iov_iter_init(&m->msg_iter, ITER_SOURCE, NULL, 0, 0);
>
>It looks like the root cause of the problem is that not all (indirect)
>callers of tipc_msg_build() properly initialize the iter.
>
>tipc_connect() is one of such caller, but AFAICS even tipc_accept() can
>reach tipc_msg_build() without proper iter initialization - via
>__tipc_sendstream -> __tipc_sendmsg.
>
>I think it's better if you address the issue in relevant callers,
>avoiding unneeded and confusing code in tipc_msg_build().
I am fully aware of callers (without initializing iovec) of this function. My intention was to make as less change as possible.
Do you think using  iov_iter_kvec() instead in the callers make sense if I go for what you suggested ?
>
>Thanks,
>
>Paolo