[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <50288778-10f6-7201-c979-bfe4635831fc@solarflare.com>
Date: Wed, 17 May 2017 15:00:04 +0100
From: Edward Cree <ecree@...arflare.com>
To: Alexei Starovoitov <ast@...com>,
David Miller <davem@...emloft.net>, <daniel@...earbox.net>
CC: <alexei.starovoitov@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] bpf: Use 1<<16 as ceiling for immediate alignment
in verifier.
On 16/05/17 23:53, Alexei Starovoitov wrote:
> following this line of thinking it feels that it should be possible
> to get rid of 'aux_off' and 'aux_off_align' and simplify the code.
> I mean we can always do
> dst_reg->min_align = min(dst_reg->min_align, src_reg->min_align);
>
> and don't use 'off' as part of alignment checks at all.
Problem with this approach, of course, is that (say) NET_IP_ALIGN +
sizeof(ethhdr) = 16 is muchly aligned, whereas if you turn all
constants into alignments you think you're only 2-byte aligned.
I think you have to track exact offsets when you can, and only turn
into an alignment when you introduce a variable.
Of course it can still be fooled by e.g. 2 + (x << 2) + 14, which it
will think is only 2-aligned when really it's 4-aligned, but unless
you want to start tracking 'bits known to be 1' as well as 'bits
known to be 0', I think you just accept that alignment tracking
isn't commutative. The obvious cases (ihl << 2 and so) will work
when written the obvious way, unless the compiler does something
perverse.
OTOH the 'track known 1s as well' might work in a nice generic way
and cover all bases, I'll have to experiment a bit with that.
-Ed
Powered by blists - more mailing lists