netdev - Re: [PATCH v2] net: qrtr: send msgs from local of same id as broadcast

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] [day] [month] [year] [list]

Message-ID: <ABYAfQB-CGWquV67Mtd-zKqb.3.1586841288308.Hmail.wenhu.wang@vivo.com>
Date:   Tue, 14 Apr 2020 13:14:48 +0800 (GMT+08:00)
From:   王文虎 <wenhu.wang@...o.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Carl Huang <cjhuang@...eaurora.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Arnd Bergmann <arnd@...db.de>,
        Nicholas Mc Guire <hofrat@...dl.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel@...o.com
Subject: Re: [PATCH v2] net: qrtr: send msgs from local of same id as broadcast


From: Bjorn Andersson <bjorn.andersson@...aro.org>
Date: 2020-04-10 03:36:00
To:  WANG Wenhu <wenhu.wang@...o.com>
Cc:  "David S. Miller" <davem@...emloft.net>,Jakub Kicinski <kuba@...nel.org>,Carl Huang <cjhuang@...eaurora.org>,
Thomas Gleixner <tglx@...utronix.de>,Arnd Bergmann <arnd@...db.de>,
Nicholas Mc Guire <hofrat@...dl.org>,netdev@...r.kernel.org,linux-kernel@...r.kernel.org,kernel@...o.com
Subject: Re: [PATCH v2] net: qrtr: send msgs from local of same id as broadcast>On Tue 07 Apr 20:32 PDT 2020, WANG Wenhu wrote:
>
>> From: Wang Wenhu <wenhu.wang@...o.com>
>> 
>> If the local node id(qrtr_local_nid) is not modified after its
>> initialization, it equals to the broadcast node id(QRTR_NODE_BCAST).
>> So the messages from local node should not be taken as broadcast
>> and keep the process going to send them out anyway.
>> 
>> The definitions are as follow:
>> static unsigned int qrtr_local_nid = NUMA_NO_NODE;
>> 
>> Fixes: commit fdf5fd397566 ("net: qrtr: Broadcast messages only from control port")
>> Signed-off-by: Wang Wenhu <wenhu.wang@...o.com>
>> ---
>> Changlog:
>>  - For coding style, line up the newline of the if conditional judgement
>>    with the one exists before.
>> 
>>  net/qrtr/qrtr.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
>> index 5a8e42ad1504..545a61f8ef75 100644
>> --- a/net/qrtr/qrtr.c
>> +++ b/net/qrtr/qrtr.c
>> @@ -907,20 +907,21 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
>>  
>>  	node = NULL;
>>  	if (addr->sq_node == QRTR_NODE_BCAST) {
>> -		enqueue_fn = qrtr_bcast_enqueue;
>> -		if (addr->sq_port != QRTR_PORT_CTRL) {
>> +		if (addr->sq_port != QRTR_PORT_CTRL &&
>> +			qrtr_local_nid != QRTR_NODE_BCAST) {
>
>So this would mean that if local_nid is configured to be the bcast
>address then rather than rejecting messages to non-control ports we will
>broadcast them.
>
>What happens when some other node in the network replies? Wouldn't it be
>better to explicitly prohibit usage of the bcast address as our node
>address?

>>
>That said, in torvalds/master qrtr_local_nid is no longer initialized to
>QRTR_NODE_BCAST, but 1. So I don't think you need this patch anymore.
>
>Regards,
>Bjorn

>

Hi Bjorn,
You are right. I see the patch that modified the nid to 1 in mainline v5.7-rc1,
and it is better to solve all the problems. As for this patch, the situation is that
I want to use qrtr in kernel to do something else(to develop another driver I name
it RPMON: Remote Processor Monitor), but the ns or service-route functionality
had been missing, so I write another file qsr.c as you had commetted which
did the same thing with ns.c.

The bad thing was I missed the patch from Manivannan.
So, anyway, this patch is not needed anymore.

Thanks,
Wenhu

>>  			release_sock(sk);
>>  			return -ENOTCONN;
>>  		}
>> +		enqueue_fn = qrtr_bcast_enqueue;
>>  	} else if (addr->sq_node == ipc->us.sq_node) {
>>  		enqueue_fn = qrtr_local_enqueue;
>>  	} else {
>> -		enqueue_fn = qrtr_node_enqueue;
>>  		node = qrtr_node_lookup(addr->sq_node);
>>  		if (!node) {
>>  			release_sock(sk);
>>  			return -ECONNRESET;
>>  		}
>> +		enqueue_fn = qrtr_node_enqueue;
>>  	}
>>  
>>  	plen = (len + 3) & ~3;
>> -- 
>> 2.17.1
>>