netdev - Re: [PATCH ipsec-next v1 8/8] iptfs: impl: add new iptfs xfrm mode impl

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: <070C751F-0BFE-4F18-B320-63786B56C56B@chopps.org>
Date: Thu, 29 Feb 2024 04:12:17 -0500
From: Christian Hopps <chopps@...pps.org>
To: Simon Horman <horms@...nel.org>
Cc: Christian Hopps <chopps@...pps.org>,
 devel@...ux-ipsec.org,
 Steffen Klassert <steffen.klassert@...unet.com>,
 netdev@...r.kernel.org
Subject: Re: [PATCH ipsec-next v1 8/8] iptfs: impl: add new iptfs xfrm mode
 impl



> On Feb 26, 2024, at 15:57, Simon Horman <horms@...nel.org> wrote:
> 
> On Thu, Feb 22, 2024 at 03:23:36PM -0500, Christian Hopps wrote:
>> 
>> Simon Horman <horms@...nel.org> writes:
>> 
>>> On Mon, Feb 19, 2024 at 03:57:35AM -0500, Christian Hopps wrote:
>>>> From: Christian Hopps <chopps@...n.net>
...
>>>> +/**
>>>> + * skb_head_to_frag() - initialize a skb_frag_t based on skb head data
>>>> + * @skb: skb with the head data
>>>> + * @frag: frag to initialize
>>>> + */
>>>> +static void skb_head_to_frag(const struct sk_buff *skb, skb_frag_t *frag)
>>>> +{
>>>> + struct page *page = virt_to_head_page(skb->data);
>>>> + unsigned char *addr = (unsigned char *)page_address(page);
>>>> +
>>>> + BUG_ON(!skb->head_frag);
>>> 
>>> Is it strictly necessary to crash the Kernel here?
>>> Likewise, many other places in this patch.
>> 
>> In all use cases it represents a programming error (bug) if the condition is met.
>> 
>> What is the correct use of BUG_ON?
> 
> Hi Christian,
> 
> I would say that BUG_ON should used in situations where
> there is an unrecoverable error to the extent where
> the entire system cannot continue to function.

Well in these cases it means that IPsec/IPTFS is in an unrecoverable state and broken. It's hard to predict how much that means "entire" to the user that expects their IPsec tunnels to be working, it may be the entire purpose of the box it's running on, so normally I don't think it's wise to try.

If you still object I will remove them.

Thanks,
Chris.

> 
> ...