netdev - Re: [PATCH bpf v2 3/4] bpf: Use EOPNOTSUPP in bpf_tcp_check

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: <fefefc43-1912-c1e5-7f50-76f5f68f9386@nvidia.com>
Date:   Mon, 31 Jan 2022 15:37:32 +0200
From:   Maxim Mikityanskiy <maximmi@...dia.com>
To:     John Fastabend <john.fastabend@...il.com>
CC:     <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        "Daniel Borkmann" <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...dia.com>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        "Jakub Kicinski" <kuba@...nel.org>,
        Petar Penkov <ppenkov@...gle.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH bpf v2 3/4] bpf: Use EOPNOTSUPP in bpf_tcp_check_syncookie

On 2022-01-25 09:06, John Fastabend wrote:
> Maxim Mikityanskiy wrote:
>> When CONFIG_SYN_COOKIES is off, bpf_tcp_check_syncookie returns
>> ENOTSUPP. It's a non-standard and deprecated code. The related function
>> bpf_tcp_gen_syncookie and most of the other functions use EOPNOTSUPP if
>> some feature is not available. This patch changes ENOTSUPP to EOPNOTSUPP
>> in bpf_tcp_check_syncookie.
>>
>> Fixes: 399040847084 ("bpf: add helper to check for a valid SYN cookie")
>> Signed-off-by: Maxim Mikityanskiy <maximmi@...dia.com>
>> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
> 
> This came up in another thread? Or was it the same and we lost the context
> in the commit msg. Either way I don't think we should start one-off
> changing these user facing error codes. Its not the only spot we do this
> and its been this way for sometime.
> 
> Is it causing a real problem?

I'm not aware of anyone complaining about it. It's just a cleanup to use 
the proper error code, since ENOTSUPP is a non-standard one (used in 
NFS?), for example, strerror() returns "Unknown error 524" instead of 
"Operation not supported".

Source: Documentation/dev-tools/checkpatch.rst:

 > ENOTSUPP is not a standard error code and should be avoided in new
 > patches. EOPNOTSUPP should be used instead.
 >
 > See: https://lore.kernel.org/netdev/20200510182252.GA411829@lunn.ch/

>> ---
>>   net/core/filter.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 780e635fb52a..2c9106704821 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -6814,7 +6814,7 @@ BPF_CALL_5(bpf_tcp_check_syncookie, struct sock *, sk, void *, iph, u32, iph_len
>>   
>>   	return -ENOENT;
>>   #else
>> -	return -ENOTSUPP;
>> +	return -EOPNOTSUPP;
>>   #endif
>>   }
>>   
>> -- 
>> 2.30.2
>>