netdev - Re: [PATCH] openvswitch: Add packet truncation 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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 9 Jun 2016 15:04:49 -0700
From:	pravin shelar <pshelar@....org>
To:	William Tu <u9012063@...il.com>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH] openvswitch: Add packet truncation support.

On Thu, Jun 9, 2016 at 1:02 PM, William Tu <u9012063@...il.com> wrote:
>
>> >
>> >         /* The only known usage of sample action is having a single
>> > user-space
>> > +        * action, or having a truncate action followed by a single
>> > user-space
>> >          * action. Treat this usage as a special case.
>> >          * The output_userspace() should clone the skb to be sent to the
>> >          * user space. This skb will be consumed by its caller.
>> >          */
>> > +       if (unlikely(nla_type(a) == OVS_ACTION_ATTR_TRUNC)) {
>> > +               struct ovs_action_trunc *trunc = nla_data(a);
>> > +
>> > +               OVS_CB(skb)->cutlen = skb->len > trunc->max_len ?
>> > +                                               skb->len -
>> > trunc->max_len : 0;
>>
>> There is no need to initialize the cutlen to zero if max-len is
>> greater then skb len. it should be already zero.
>>
>> > +               a = nla_next(a, &rem);
>> > +       }
>> > +
>> There cutlen is set in skb cb, but next action could be other than
>> userspace, in that case cutlen is leaked to remain action list.
>>
>
> Thanks! I will fix it.
>
> Thinking about cutlen to the remaining action list, should we consider the
> truncate and output in sample action? ex:  "sample(truncate(64), output:2)"
>
The patch already covers it. sample calls add_deferred_actions() which
would execute nested set of actions later on.