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.

It looks to me that the minimum length is:

skb_vlan_tag_present(skb) ? offsetof(struct vlan_ethhdr, h_vlan_proto) : 0

And consider the vlan tag insertion done in tun_put_user(), we need trim 4 more bytes if vlan tag is present.

Thanks

Reply via email to