Re: [PATCH 08/10] netfilter: ebtables: try native set/getsockopt handlers, too

[Florian Westphal <fw@strlen.de>]

Patrick McHardy <kaber@trash.net> wrote:
> > ebtables can be compiled to perform userspace-side padding of
> > structures. In that case, all the structures are already in the
> > 'native' format expected by the kernel.
> > 
> > This tries to determine what format the userspace program is
> > using.
> > 
> > For most set/getsockopts, this can be done by re-trying the
> > native handler once the compat_ version returns an error.
> > 
> > In case of EBT_SO_GET_ENTRIES, the native handler is tried first,
> > it should error out very early when checking the *len argument
> > (the compat version has to defer this check until after
> >  iterating over the kernel data set once, to adjust for all
> >  the structure size differences).
> 
> Can't we place an explicit check somewhere instead of "probing"
> for compatibility? Checking the size of struct ebt_replace for
> compat tasks should be suitable.

Yes, thats what is being done where possible.
Here is a list of the set/getsockopts and the current "detect" strategy:

EBT_SO_SET_COUNTERS:
in compat handler, copy struct ebt_replace from user, check that
len == sizeof(struct compat_ebt_replace) + repl.num_counters * sizeof(struct ebt_counter)

if yes: continue, else try non-compat handler

EBT_SO_SET_ENTRIES:
in compat handler, copy struct ebt_replace from user, check that
len == sizeof(struct compat_ebt_replace) + repl.entries_size

if yes, continue, else try non-compat handler.

Actually, one could move the call to the non-compat handler around
so that the non-compat handler is only called when this
particular error occurs and not on other errors, like e.g. EFAULT.

I can do this in the next version of the patch series.

EBT_SO_GET_INFO, EBT_SO_GET_INIT_INFO:
trivial: *len must be sizeof(struct compat_ebt_replace), else try
non-compat handler

EBT_SO_GET_ENTRIES, EBT_SO_GET_INIT_ENTRIES:
In the non-compat case, this check is easy:
len must be "sizeof(struct ebt_replace) + entries_size_in_kernel"
(and size of all counters, but this is irrelevant because the counters
 do not need compat fixups).

Thus in case of EBT_SO_GET_ENTRIES and EBT_SO_GET_INIT_ENTRIES
the native handler gets called first (compat_ translates
the kernel table first to determine how large the data set would be
in user space, so the check cannot be done right after copying struct
compat_ebt_replace from userspace).