On 12/9/2024 2:20 PM, Florian Westphal wrote:
> Karol Przybylski <karprzy7@xxxxxxxxx> wrote:
>
> [ CC original patch author and mass-trimming CCs ]
>
>> The comparison seclen >= 0 in net/netfilter/nfnetlink_queue.c is redundant because seclen is an unsigned value, and such comparisons are always true.
>>
>> This patch removes the unnecessary comparison replacing it with just 'greater than'
>>
>> Discovered in coverity, CID 1602243
>>
>> Signed-off-by: Karol Przybylski <karprzy7@xxxxxxxxx>
>> ---
>> net/netfilter/nfnetlink_queue.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
>> index 5110f29b2..eacb34ffb 100644
>> --- a/net/netfilter/nfnetlink_queue.c
>> +++ b/net/netfilter/nfnetlink_queue.c
>> @@ -643,7 +643,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
>>
>> if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
>> seclen = nfqnl_get_sk_secctx(entskb, &ctx);
>> - if (seclen >= 0)
>> + if (seclen > 0)
>> size += nla_total_size(seclen);
> Casey, can you please have a look?
Yes, there is indeed an issue here. I will look into the correct change today.
Thank you.
>
> AFAICS security_secid_to_secctx() could return -EFOO, so it seems
> nfqnl_get_sk_secctx has a bug and should conceal < 0 retvals
> (the function returns u32), in addition to the always-true >= check
> fixup.
