netdev - Re: [RFC PATCH 2/2] PID-based packet filtering support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALKxAXVK9EhJiNOuq0JYqWiw75q3TYCScp+bGf-qqF_irKYtbA@mail.gmail.com>
Date:	Thu, 5 Apr 2012 10:41:35 +0100
From:	Nuno Martins <nuno.martins@...xamagica.pt>
To:	chetan loke <loke.chetan@...il.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Alfredo Matos <alfredo.matos@...xamagica.pt>,
	Paulo Trezentos <paulo.trezentos@...xamagica.pt>
Subject: Re: [RFC PATCH 2/2] PID-based packet filtering support

On 4 April 2012 18:01, chetan loke <loke.chetan@...il.com> wrote:
> On Wed, Apr 4, 2012 at 5:16 AM, Nuno Martins
> <nuno.martins@...xamagica.pt> wrote:
>
>>  net/pidmonitor/Makefile           |    3 +
>
> might make sense to prefix files with 'net' tag. Like: net_pidmonitor,
> net_proc_monitor.c etc. Because proc_monitor is too generic.
>

Good point, will refactor it.

>
>> +static int is_equal_packet_info(struct packetInfo *pi,
>
> It should be 'packet_info' and not 'packetInfo'. Init-char convention
> is not used in linux. same goes for portInfo and other structs(if
> any).
>
>

Thanks for spotting that. Will be fixed in our tree.


>> +static int insert_address(struct packetInfo *lpi, struct portInfo *port_info)
>> +{
>> +
>> +       switch (lpi->protocol) {
>> +       case IPPROTO_TCP:
>> +               if (!(port_info->tcp)) {
>> +                       port_info->tcp = kmalloc(sizeof(struct local_addresses_list), GFP_KERNEL);
>
> Do you think it might make sense to pre-alloc a list? or
> managing/growing that list would add more pain/code than it's worth?
> I'm just thinking in terms of search scalability when memory is
> fragmented and we have lots of nodes in the tree.But I'm sure you
> must've done some light tests.
>
>
>> +static struct portInfo *create_packet_info(struct packetInfo *lpi)
>> +{
>> +       struct portInfo *pi = NULL;
>> +       pi = kmalloc(sizeof(*pi), GFP_KERNEL);
>> +
>> +       if (!pi)
>> +               return NULL;
>> +
>> +       pi->port = lpi->port;
>> +       pi->tcp = NULL;
>> +       pi->tcp_list_counter = 0;
>> +       pi->udp = NULL;
>> +       pi->udp_list_counter = 0;
>> +
>> +       insert_address(lpi, pi);
>> +
>> +       return pi;
>> +}
>> +


Our tests with 1024 node tree gives us something like 1.3 us on
average. Your suggestion makes a lot of sense, and we will definitely
look into.
This doesn't seem hard to do as we are considering introducing a pid
based hash table, with a tree in each entry, which can and possibly
should be pre-allocated.


>
> Just thinking out loud - what if packet_info and pid were introduced
> as part of struct sock or something? The no need to kmalloc.
> And at the end of the bind/accept/other-calls-you-intercept-via-probes
> you can populate the structs. Then the kprobes could go away too. But
> I know nothing about BPF so I can't really comment for sure.

That's also a good approach. We will look into it.

>
>
> Chetan

Thank you very much for your feedback

Nuno Martins
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html