On Thu, Jan 4, 2018 at 8:28 AM, Jason Wang <jasow...@redhat.com> wrote: > > > On 2018年01月02日 17:19, Willem de Bruijn wrote: >>>> >>>> More importantly, should this program just return a boolean pass or >>>> drop. Taking a length and trimming may introduce bugs later on if the >>>> stack parses the packet unconditionally, expecting a minimum size >>>> to be present. >>>> >>>> This was the reason for introducing sk_filter_trim_cap and using that >>>> in other sk_filter sites. >>>> >>>> A quick scan shows that tun_put_user expects a full vlan tag to exist >>>> if skb_vlan_tag_present(skb), for instance. If trimmed to below this >>>> length the final call to skb_copy_datagram_iter may have negative >>>> length. >>>> >>>> This is an issue with the existing sk_filter call as much as with the >>>> new run_ebpf_filter call. >>> >>> Good point, so consider it was used by sk_filter too, we need to fix it >>> anyway. Actually, I've considered the boolean return value but finally I >>> decide to obey the style of sk filter. Maybe the trimming has real user. >>> e.g >>> high speed header recoding/analysis? Consider it's not hard to fix, how >>> about just keep that? >> >> I don't see an obvious use case, but sure. We'll just need to look >> at what the minimum trim length needs to be. > > > Try to reproduce the possible issue, but looks like we are safe since we may > hit -EFAULT which is returned by skb_copy_datagram_iter() before.
Ah, indeed, it can handle short lengths. Great, thanks for checking. > So in V2, > I will keep the code as is except trim 4 more bytes if vlan tag is present. This is do not follow. Perhaps the patch will make it clear.
