netdev - Re: [patch iproute2 v8 1/2] lib/libnetlink: Add functions rtnl_talk_msg and rtnl_talk

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] [day] [month] [year] [list]

Message-ID: <d102796c-66ff-a4d6-d181-1abf0237089f@gmail.com>
Date:   Thu, 11 Jan 2018 17:06:03 -0700
From:   David Ahern <dsahern@...il.com>
To:     Phil Sutter <phil@....cc>, Chris Mi <chrism@...lanox.com>,
        netdev@...r.kernel.org, gerlitz.or@...il.com,
        stephen@...workplumber.org, marcelo.leitner@...il.com
Subject: Re: [patch iproute2 v8 1/2] lib/libnetlink: Add functions
 rtnl_talk_msg and rtnl_talk_iov

On 1/11/18 8:08 AM, Phil Sutter wrote:
> On Wed, Jan 10, 2018 at 09:12:45PM +0100, Phil Sutter wrote:
>> On Wed, Jan 10, 2018 at 12:20:36PM -0700, David Ahern wrote:
>> [...]
>>> 2. I am using a batch file with drop filters:
>>>
>>> filter add dev eth2 ingress protocol ip pref 273 flower dst_ip
>>> 192.168.253.0/16 action drop
>>>
>>> and for each command tc is trying to dlopen m_drop.so:
>>>
>>> open("/usr/lib/tc//m_drop.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such
>>> file or directory)
>>
>> [...]
>>
>>> Can you look at a follow on patch (not part of this set) to cache status
>>> of dlopen attempts?
>>
>> IMHO the logic used in get_action_kind() for gact is the culprit here:
>> After trying to dlopen m_drop.so, it dlopens m_gact.so although it is
>> present already. (Unless I missed something.)
> 
> Not quite, m_gact.c is statically compiled in and there is logic around
> dlopen(NULL, ...) to prevent calling it twice.
> 
>> I guess the better (and easier) fix would be to create some more struct
>> action_util instances in m_gact.c for the primitives it supports so that
>> the lookup in action_list succeeds for consecutive uses. Note that
>> parse_gact() even supports this already.
> 
> Sadly, this doesn't fly: If a lookup for action 'drop' is successful,
> that value is set as TCA_ACT_KIND and the kernel doesn't know about it.
> 
> I came up with an alternative solution, what do you think about attached
> patch?

Looks ok to me and removes the repeated open overhead. Send it formally
and cc Jiri and Jamal.

Thanks,