netdev - Re: [net v2 2/7] ch_ktls: Correction in finding correct length

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]

Date:   Fri, 23 Oct 2020 18:58:44 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Rohit Maheshwari <rohitm@...lsio.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, secdev@...lsio.com
Subject: Re: [net v2 2/7] ch_ktls: Correction in finding correct length

On Fri, 23 Oct 2020 11:01:29 +0530 Rohit Maheshwari wrote:
> There is a possibility of linear skbs coming in. Correcting
> the length extraction logic.
> 
> Fixes: 5a4b9fe7fece ("cxgb4/chcr: complete record tx handling")
> Signed-off-by: Rohit Maheshwari <rohitm@...lsio.com>

> @@ -980,6 +979,10 @@ chcr_ktls_write_tcp_options(struct chcr_ktls_info *tx_info, struct sk_buff *skb,
>  		return NETDEV_TX_BUSY;
>  	}
>  
> +	if (unlikely(credits < ETHTXQ_STOP_THRES)) {
> +		chcr_eth_txq_stop(q);
> +		wr_mid |= FW_WR_EQUEQ_F | FW_WR_EQUIQ_F;
> +	}
>  	pos = &q->q.desc[q->q.pidx];
>  	wr = pos;
>  
> @@ -987,42 +990,51 @@ chcr_ktls_write_tcp_options(struct chcr_ktls_info *tx_info, struct sk_buff *skb,
>  	wr->op_immdlen = htonl(FW_WR_OP_V(FW_ETH_TX_PKT_WR) |
>  			       FW_WR_IMMDLEN_V(ctrl));
>  
> -	wr->equiq_to_len16 = htonl(FW_WR_LEN16_V(len16));
> +	wr->equiq_to_len16 = htonl(wr_mid | FW_WR_LEN16_V(len16));
>  	wr->r3 = 0;
>  
>  	cpl = (void *)(wr + 1);
>  
>  	/* CPL header */
> -	cpl->ctrl0 = htonl(TXPKT_OPCODE_V(CPL_TX_PKT) | TXPKT_INTF_V(tx_chan) |
> +	cpl->ctrl0 = htonl(TXPKT_OPCODE_V(CPL_TX_PKT) |
> +			   TXPKT_INTF_V(tx_info->tx_chan) |
>  			   TXPKT_PF_V(tx_info->adap->pf));
>  	cpl->pack = 0;
>  	cpl->len = htons(pktlen);
> -	/* checksum offload */
> -	cpl->ctrl1 = 0;
> -
> -	pos = cpl + 1;
>  
>  	memcpy(buf, skb->data, pktlen);
>  	if (tx_info->ip_family == AF_INET) {
>  		/* we need to correct ip header len */
>  		ip = (struct iphdr *)(buf + maclen);
>  		ip->tot_len = htons(pktlen - maclen);
> +		cntrl1 = TXPKT_CSUM_TYPE_V(TX_CSUM_TCPIP);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	} else {
>  		ip6 = (struct ipv6hdr *)(buf + maclen);
>  		ip6->payload_len = htons(pktlen - maclen - iplen);
> +		cntrl1 = TXPKT_CSUM_TYPE_V(TX_CSUM_TCPIP6);
>  #endif
>  	}
> +
> +	cntrl1 |= T6_TXPKT_ETHHDR_LEN_V(maclen - ETH_HLEN) |
> +		  TXPKT_IPHDR_LEN_V(iplen);
> +	/* checksum offload */
> +	cpl->ctrl1 = cpu_to_be64(cntrl1);
> +
> +	pos = cpl + 1;
> +
>  	/* now take care of the tcp header, if fin is not set then clear push
>  	 * bit as well, and if fin is set, it will be sent at the last so we
>  	 * need to update the tcp sequence number as per the last packet.
>  	 */
>  	tcp = (struct tcphdr *)(buf + maclen + iplen);
>  
> -	if (!tcp->fin)
> +	if (!fin) {
>  		tcp->psh = 0;
> -	else
> +		tcp->fin = 0;
> +	} else {
>  		tcp->seq = htonl(tx_info->prev_seq);
> +	}
>  
>  	chcr_copy_to_txd(buf, &q->q, pos, pktlen);
>  

> @@ -1905,8 +1920,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
>  	cxgb4_reclaim_completed_tx(adap, &q->q, true);
>  	/* if tcp options are set but finish is not send the options first */
>  	if (!th->fin && chcr_ktls_check_tcp_options(th)) {
> -		ret = chcr_ktls_write_tcp_options(tx_info, skb, q,
> -						  tx_info->tx_chan);
> +		ret = chcr_ktls_write_tcp_options(tx_info, skb, q, false);
>  		if (ret)
>  			return NETDEV_TX_BUSY;
>  	}

>  	/* tcp finish is set, send a separate tcp msg including all the options
>  	 * as well.
>  	 */
>  	if (th->fin)
> -		chcr_ktls_write_tcp_options(tx_info, skb, q, tx_info->tx_chan);
> +		chcr_ktls_write_tcp_options(tx_info, skb, q, true);
>  
>  out:
>  	dev_kfree_skb_any(skb);

I don't see how these changes related to a linear skb.