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  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]
Date:	Thu, 19 Mar 2015 11:29:32 -0400
From:	Willem de Bruijn <willemb@...gle.com>
To:	Alexander Drozdov <al.drozdov@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Daniel Borkmann <dborkman@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Network Development <netdev@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] af_packet: pass checksum validation status to the user

On Thu, Mar 19, 2015 at 4:01 AM, Alexander Drozdov <al.drozdov@...il.com> wrote:
> Introduce TP_STATUS_CSUM_VALID tp_status flag to tell the
> af_packet user that at least the transport header checksum
> has been already validated.

This changes the interface slightly. Processes should be treating
this previously unused bit as reserved and other flags have
been added to the bitmap in this manner as well, so this should
then be safe here, too.

> For now, the flag may be set for incoming packets only.

Why? You cannot change the semantics of the flag afterwards.
Better to support both directions from the start.

> Signed-off-by: Alexander Drozdov <al.drozdov@...il.com>
> ---
>  include/uapi/linux/if_packet.h | 1 +
>  net/packet/af_packet.c         | 9 +++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
> index da2d668..053bd10 100644
> --- a/include/uapi/linux/if_packet.h
> +++ b/include/uapi/linux/if_packet.h
> @@ -99,6 +99,7 @@ struct tpacket_auxdata {
>  #define TP_STATUS_VLAN_VALID           (1 << 4) /* auxdata has valid tp_vlan_tci */
>  #define TP_STATUS_BLK_TMO              (1 << 5)
>  #define TP_STATUS_VLAN_TPID_VALID      (1 << 6) /* auxdata has valid tp_vlan_tpid */
> +#define TP_STATUS_CSUM_VALID           (1 << 7)
>
>  /* Tx ring - header status */
>  #define TP_STATUS_AVAILABLE          0
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 6ecf8dd..3f09dda 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1918,6 +1918,10 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
>
>         if (skb->ip_summed == CHECKSUM_PARTIAL)
>                 status |= TP_STATUS_CSUMNOTREADY;
> +       else if (skb->pkt_type != PACKET_OUTGOING &&
> +                (skb->ip_summed == CHECKSUM_COMPLETE ||
> +                 skb_csum_unnecessary(skb)))
> +               status |= TP_STATUS_CSUM_VALID;
>
>         if (snaplen > res)
>                 snaplen = res;
> @@ -3015,6 +3019,11 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
>                 aux.tp_status = TP_STATUS_USER;
>                 if (skb->ip_summed == CHECKSUM_PARTIAL)
>                         aux.tp_status |= TP_STATUS_CSUMNOTREADY;
> +               else if (skb->pkt_type != PACKET_OUTGOING &&
> +                        (skb->ip_summed == CHECKSUM_COMPLETE ||
> +                         skb_csum_unnecessary(skb)))
> +                       aux.tp_status |= TP_STATUS_CSUM_VALID;
> +

These two sections are near duplicates. I'd move the entire status
initialization, including existing TP_STATUS_USER and
TP_STATUS_CSUMNOTREADY fields to a helper function.

It's a bit unfortunately that we have to use an extra bit to add a signal
that is a near inverse of the existing TP_STATUS_CSUMNOTREADY
(bar CHECKSUM_NONE). I do not immediately see a better way,
either, though. And tp_status has plenty room at 32 bits.

>                 aux.tp_len = PACKET_SKB_CB(skb)->origlen;
>                 aux.tp_snaplen = skb->len;
>                 aux.tp_mac = 0;
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists