netdev - RE: [Crypto v6 03/12] tls: support for inline tls

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] [day] [month] [year] [list]

Message-ID: <CY4PR1201MB02308690FA03CD884C60836B97CF0@CY4PR1201MB0230.namprd12.prod.outlook.com>
Date:   Tue, 20 Feb 2018 16:42:40 +0000
From:   Atul Gupta <atul.gupta@...lsio.com>
To:     David Miller <davem@...emloft.net>
CC:     "davejwatson@...com" <davejwatson@...com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "sd@...asysnail.net" <sd@...asysnail.net>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ganesh GR <ganeshgr@...lsio.com>
Subject: RE: [Crypto v6 03/12] tls: support for inline tls



-----Original Message-----
From: linux-crypto-owner@...r.kernel.org [mailto:linux-crypto-owner@...r.kernel.org] On Behalf Of David Miller
Sent: Tuesday, February 20, 2018 9:46 PM
To: Atul Gupta <atul.gupta@...lsio.com>
Cc: davejwatson@...com; herbert@...dor.apana.org.au; sd@...asysnail.net; linux-crypto@...r.kernel.org; netdev@...r.kernel.org; Ganesh GR <ganeshgr@...lsio.com>
Subject: Re: [Crypto v6 03/12] tls: support for inline tls

From: Atul Gupta <atul.gupta@...lsio.com>
Date: Mon, 19 Feb 2018 12:19:41 +0530

> +	struct net_device *netdev = NULL;
> +
> +	netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);

No need for an assignment in the variable declaration here.
You immediately set it to something else unconditionally.
Sure.

> +static int get_tls_offload_dev(struct sock *sk) {
> +	struct net_device *netdev;
> +	struct tls_device *dev;
> +	int rc = 0;
> +
> +	netdev = get_netdev(sk);
> +	if (!netdev)
> +		return -EINVAL;
> +
> +	mutex_lock(&device_mutex);
> +	list_for_each_entry(dev, &device_list, dev_list) {
> +		if (dev->netdev && dev->netdev(dev, netdev)) {
> +			rc = -EEXIST;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&device_mutex);
> +	dev_put(netdev);
> +	return rc;
> +}

This is really a confusing function.

It's name suggests that it "gets" the offload device.  In that case, if it is found it should return success.  Instead we get an -EEXIST error in that case.  And it returns 0 if not found.
Will rename to no_tls_offload_dev, return 0 is used as default condition in calling function.

Better to make this do what it says it does, which would be to return '0' when the device is found and return -ENODEV when it is not found.

> +	tcp_prot.unhash(sk);
Done

Do not force this to the ipv4 TCP instance, use the pointer through the socket to call the proper unhash method.

> +	err = tcp_prot.hash(sk);
Done

Likewise.
Thanks