netdev - Re: [PATCH net-next v2 1/3] drop_monitor: Implement namespace filtering/reporting for software drops

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]

Date:   Wed, 23 Nov 2022 17:21:23 +0200
From:   nb <nikolay.borisov@...tuozzo.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     nhorman@...driver.com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, netdev@...r.kernel.org, kernel@...tuozzo.com
Subject: Re: [PATCH net-next v2 1/3] drop_monitor: Implement namespace
 filtering/reporting for software drops



On 23.11.22 г. 17:16 ч., Ido Schimmel wrote:
> On Wed, Nov 23, 2022 at 04:28:15PM +0200, Nikolay Borisov wrote:
>>   static void trace_drop_common(struct sk_buff *skb, void *location)
>>   {
>>   	struct net_dm_alert_msg *msg;
>> @@ -219,7 +233,11 @@ static void trace_drop_common(struct sk_buff *skb, void *location)
>>   	int i;
>>   	struct sk_buff *dskb;
>>   	struct per_cpu_dm_data *data;
>> -	unsigned long flags;
>> +	unsigned long flags, ns_id = 0;
>> +
>> +	if (skb->dev && net_dm_ns &&
>> +	    dev_net(skb->dev)->ns.inum != net_dm_ns)
> 
> I don't think this is going to work, unfortunately. 'skb->dev' is in a
> union with 'dev_scratch' so 'skb->dev' does not necessarily point to a
> valid netdev at all times. It can explode when dev_net() tries to
> dereference it.
> 
> __skb_flow_dissect() is doing something similar, but I believe there the
> code paths were audited to make sure it is safe.
> 
> Did you consider achieving this functionality with a BPF program
> attached to skb::kfree_skb tracepoint? I believe BPF programs are run
> with page faults disabled, so it should be safe to attempt this there.

How would that be different than the trace_drop_common which is called 
as part of the trace_kfree_skb, as it's really passed as trace point 
probe via:

net_dm_trace_on_set->register_trace_kfree_skb(trace_drop_common)