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: <3e017f77-87dd-78e1-321d-90c3e57a68d9@quicinc.com>
Date: Tue, 26 Dec 2023 16:20:03 -0800
From: Chris Lew <quic_clew@...cinc.com>
To: Simon Horman <horms@...nel.org>, Sarannya S <quic_sarannya@...cinc.com>
CC: <quic_bjorande@...cinc.com>, <andersson@...nel.org>,
        <mathieu.poirier@...aro.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>,
        Manivannan Sadhasivam <mani@...nel.org>,
        "David S. Miller"
	<davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "open list:NETWORKING
 [GENERAL]" <netdev@...r.kernel.org>
Subject: Re: [PATCH V1] net: qrtr: ns: Ignore ENODEV failures in ns



On 12/23/2023 5:56 AM, Simon Horman wrote:
> [Dropped bjorn.andersson@...nel.org, as the correct address seems
>   to be andersson@...nel.org, which is already in the CC list.
>   kernel.org rejected sending this email without that update.]
> 
> On Thu, Dec 21, 2023 at 03:36:50PM +0530, Sarannya S wrote:
>> From: Chris Lew <quic_clew@...cinc.com>
>>
>> Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
>> indicate that either the local port has been closed or the remote has
>> gone down. Neither of these scenarios are fatal and will eventually be
>> handled through packets that are later queued on the control port.
>>
>> Signed-off-by: Chris Lew <quic_clew@...cinc.com>
>> Signed-off-by: Sarannya Sasikumar <quic_sarannya@...cinc.com>
>> ---
>>   net/qrtr/ns.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
>> index abb0c70..8234339 100644
>> --- a/net/qrtr/ns.c
>> +++ b/net/qrtr/ns.c
>> @@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr *dest,
>>   	msg.msg_namelen = sizeof(*dest);
>>   
>>   	ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
>> -	if (ret < 0)
>> +	if (ret < 0 && ret != -ENODEV)
>>   		pr_err("failed to announce del service\n");
>>   
>>   	return ret;
> 
> Hi,
> 
> The caller of service_announce_del() ignores it's return value.
> So the only action on error is the pr_err() call above, and so
> with this patch -ENODEV is indeed ignored.
> 
> However, I wonder if it would make things clearer to the reader (me?)
> if the return type of service_announce_del was updated void. Because
> as things stand -ENODEV may be returned, which implies something might
> handle that, even though it doe not.
> 
> The above notwithstanding, this change looks good to me.
> 
> Reviewed-by: Simon Horman <horms@...nel.org>
> 
> ...

Hi Simon, thanks for the review and suggestion. We weren't sure whether 
we should change the function prototype on these patches on the chance 
that there will be something that listens and handles this in the 
future. I think it's a good idea to change it to void and we can change 
it back if there is such a usecase in the future.

Powered by blists - more mailing lists