netdev - Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878uxbjl4n.wl%atzm@stratosphere.co.jp>
Date:	Wed, 30 Oct 2013 16:03:36 +0900
From:	Atzm Watanabe <atzm@...atosphere.co.jp>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
	<stephen@...workplumber.org>
Subject: Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace

At Mon, 28 Oct 2013 20:11:14 +0000,
Ben Hutchings wrote:
> 
> On Fri, 2013-10-25 at 18:59 -0400, David Miller wrote:
> > From: Atzm Watanabe <atzm@...atosphere.co.jp>
> > Date: Tue, 22 Oct 2013 17:39:40 +0900
> > 
> > >  struct tpacket_hdr_variant1 {
> > >  	__u32	tp_rxhash;
> > >  	__u32	tp_vlan_tci;
> > > +	__u32	tp_vlan_tpid;
> > >  };
> > >  
> > 
> > You cannot do this, the header length is not variable.
> 
> Well it is variable to an extent.  This is used as a member of the final
> anonymous union member of struct tpacket3_hdr, and the latter is
> specified to be tail-padded to a multiple of 16 bytes
> (TPACKET_ALIGNMENT).  Since its current size is 36 bytes, I think it can
> safely grow by 12 bytes, so long as userland doesn't depend on
> getsockopt(..., PACKET_HDRLEN, ...) returning only specific values.
> 
> Possibly there should be a separate struct tpacket_hdr_variant2 which
> includes the extra member.  Possibly there should also be a status flag
> to indicate that this member is present.

Should it be structures as below?

struct tpacket_hdr_variant1 {
        __u32   tp_rxhash;
        __u32   tp_vlan_tci;
};

struct tpacket_hdr_variant2 {
        __u32   tp_rxhash;
        __u32   tp_vlan_tci;
        __u32   tp_vlan_tpid;
};

struct tpacket3_hdr {
        __u32           tp_next_offset;
        __u32           tp_sec;
        __u32           tp_nsec;
        __u32           tp_snaplen;
        __u32           tp_len;
        __u32           tp_status;
        __u16           tp_mac;
        __u16           tp_net;
        /* pkt_hdr variants */
        union {
                struct tpacket_hdr_variant1 hv1;
                struct tpacket_hdr_variant2 hv2;
        };
};

If it's ok, I'd like to send the patch v2.


> > This patch has been submitted several times, each of which you
> > have been shown ways in which your changes break userspace in
> > one way or another.
> 
> I think we established that struct tpacket3_hdr can't grow arbitrarily,
> but not that it can't grow at all.

I think so, too.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html