Re: [PATCH v2] netfilter: properly initialize xt_table_info structure

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, May 31, 2018 at 01:55:57PM +0200, Michal Kubecek wrote:
> On Thu, May 31, 2018 at 01:32:16PM +0200, Michal Kubecek wrote:
> > I think I start to understand the problem. IPT_SO_GET_ENTRIES leads to
> > calling copy_entries_to_user() which copies the entries as they are to
> > user provided buffer. It also copies instances of struct xt_entry_match
> > and struct xt_entry_target which contain kernel pointers. We then
> > rewrite them with match/target name for userspace but the layout looks
> > (on x86_64) like this
> > 
> > /* offset    |  size */  type = struct xt_entry_match {
> > /*    0      |    32 */    union {
> > /*                32 */        struct {
> > /*    0      |     2 */            __u16 match_size;
> > /*    2      |    29 */            char name[29];
> > /*   31      |     1 */            __u8 revision;
> > 
> >                                    /* total size (bytes):   32 */
> >                                } user;
> > /*                16 */        struct {
> > /*    0      |     2 */            __u16 match_size;
> > /* XXX  6-byte hole  */
> > /*    8      |     8 */            struct xt_match *match;
> > 
> >                                    /* total size (bytes):   16 */
> >                                } kernel;
> > /*                 2 */        __u16 match_size;
> > 
> >                                /* total size (bytes):   32 */
> >                            } u;
> > /*   32      |     0 */    unsigned char data[];
> > 
> >                            /* total size (bytes):   32 */
> >                          }
> > 
> > 
> > so that if match name is no longer than five characters (which is often
> > the case), writing to .u.user.name leaves .u.kernel.match untouched. The
> > same problem exists in struct xt_entry_target.
> 
> And this should no longer happen since the series
> 
>  f32815d21d4d ("xtables: add xt_match, xt_target and data copy_to_user functions")
>  f77bc5b23fb1 ("iptables: use match, target and data copy_to_user helpers")
>  e47ddb2c4691 ("ip6tables: use match, target and data copy_to_user helpers")
>  244b531bee2b ("arptables: use match, target and data copy_to_user helpers")
>  b5040f6c33a5 ("ebtables: use match, target and data copy_to_user helpers")
>  4915f7bbc402 ("xtables: use match, target and data copy_to_user helpers in compat")
>  ec2318904965 ("xtables: extend matches and targets with .usersize")
> 
> changed the logic in 4.11-rc1.

Thank you so much for the detailed description.  And sorry for digging
up this old issue.  Peter, if you could verify that you do not see this
issue on a kernel newer than 4.11, that would be wonderful.

Michal, do you think it is worth backporting those commits to the 4.9.y
and 4.4.y stable kernels to remove this problem there?

thanks,

greg k-h