[PATCH v2 0/5] build warnings cleanup

[Atul Gupta]

Build warnings cleanup reported for
- using only 128b key
- wait for buffer in sendmsg/sendpage
- check for null before using skb
- free rspq_skb_cache in error path
- indentation

v2:
  Added bug report description for 0002
  Incorported comments from Dan Carpenter

Atul Gupta (5):
  crypto:chtls: key len correction
  crypto: chtls: wait for memory sendmsg, sendpage
  crypto: chtls: dereference null variable
  crypto: chtls: kbuild warnings
  crypto: chtls: free beyond end rspq_skb_cache

 drivers/crypto/chelsio/chtls/chtls.h  |   1 +
 drivers/crypto/chelsio/chtls/chtls_hw.c   |   6 +-
 drivers/crypto/chelsio/chtls/chtls_io.c   | 104 +++---
 drivers/crypto/chelsio/chtls/chtls_main.c |   3 +-
 4 files changed, 98 insertions(+), 16 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/5] crypto:chtls: key len correction

[Atul Gupta]

corrected the key length to copy 128b key. Removed 192b and 256b
key as user input supports key of size 128b in gcm_ctx

Reported-by: Dan Carpenter 
Signed-off-by: Atul Gupta 
---
 drivers/crypto/chelsio/chtls/chtls_hw.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_hw.c 
b/drivers/crypto/chelsio/chtls/chtls_hw.c
index 54a13aa9..55d5014 100644
--- a/drivers/crypto/chelsio/chtls/chtls_hw.c
+++ b/drivers/crypto/chelsio/chtls/chtls_hw.c
@@ -213,7 +213,7 @@ static int chtls_key_info(struct chtls_sock *csk,
  struct _key_ctx *kctx,
  u32 keylen, u32 optname)
 {
-   unsigned char key[CHCR_KEYCTX_CIPHER_KEY_SIZE_256];
+   unsigned char key[AES_KEYSIZE_128];
struct tls12_crypto_info_aes_gcm_128 *gcm_ctx;
unsigned char ghash_h[AEAD_H_SIZE];
struct crypto_cipher *cipher;
@@ -228,10 +228,6 @@ static int chtls_key_info(struct chtls_sock *csk,
 
if (keylen == AES_KEYSIZE_128) {
ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_128;
-   } else if (keylen == AES_KEYSIZE_192) {
-   ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_192;
-   } else if (keylen == AES_KEYSIZE_256) {
-   ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_256;
} else {
pr_err("GCM: Invalid key length %d\n", keylen);
return -EINVAL;
-- 
1.8.3.1

[PATCH v2 5/5] crypto: chtls: free beyond end rspq_skb_cache

[Atul Gupta]

Reported-by: Dan Carpenter 
Signed-off-by: Atul Gupta 
---
 drivers/crypto/chelsio/chtls/chtls_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c 
b/drivers/crypto/chelsio/chtls/chtls_main.c
index 273afd3..9b07f91 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -250,7 +250,7 @@ static void *chtls_uld_add(const struct cxgb4_lld_info 
*info)
 
return cdev;
 out_rspq_skb:
-   for (j = 0; j <= i; j++)
+   for (j = 0; j < i; j++)
kfree_skb(cdev->rspq_skb_cache[j]);
kfree_skb(cdev->askb);
 out_skb:
-- 
1.8.3.1

[PATCH v2 2/5] crypto: chtls: wait for memory sendmsg, sendpage

[Atul Gupta]

address suspicious code 

1210   set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
1211   }

The issue is that in the code above, set_bit is never reached
due to the 'continue' statement at line 1208.

Also reported by bug report:
1210   set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
   ^^^
Not reachable.

Its required to wait for buffer in the send path and takes care of
unaddress and un-handled SOCK_NOSPACE.

v2: use csk_mem_free where appropriate
proper indent of goto do_nonblock
replace out with do_rm_wq

Reported-by: Gustavo A. R. Silva 
Reported-by: Dan Carpenter 
Signed-off-by: Atul Gupta 
---
 drivers/crypto/chelsio/chtls/chtls.h  |  1 +
 drivers/crypto/chelsio/chtls/chtls_io.c   | 90 +--
 drivers/crypto/chelsio/chtls/chtls_main.c |  1 +
 3 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls.h 
b/drivers/crypto/chelsio/chtls/chtls.h
index 1b2f43c..a53a0e6 100644
--- a/drivers/crypto/chelsio/chtls/chtls.h
+++ b/drivers/crypto/chelsio/chtls/chtls.h
@@ -144,6 +144,7 @@ struct chtls_dev {
struct list_head rcu_node;
struct list_head na_node;
unsigned int send_page_order;
+   int max_host_sndbuf;
struct key_map kmap;
 };
 
diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c 
b/drivers/crypto/chelsio/chtls/chtls_io.c
index 840dd01..7aa5d90 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -914,6 +914,78 @@ static u16 tls_header_read(struct tls_hdr *thdr, struct 
iov_iter *from)
return (__force u16)cpu_to_be16(thdr->length);
 }
 
+static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
+{
+   return (cdev->max_host_sndbuf - sk->sk_wmem_queued);
+}
+
+static int csk_wait_memory(struct chtls_dev *cdev,
+  struct sock *sk, long *timeo_p)
+{
+   DEFINE_WAIT_FUNC(wait, woken_wake_function);
+   int sndbuf, err = 0;
+   long current_timeo;
+   long vm_wait = 0;
+   bool noblock;
+
+   current_timeo = *timeo_p;
+   noblock = (*timeo_p ? false : true);
+   sndbuf = cdev->max_host_sndbuf;
+   if (csk_mem_free(cdev, sk)) {
+   current_timeo = (prandom_u32() % (HZ / 5)) + 2;
+   vm_wait = (prandom_u32() % (HZ / 5)) + 2;
+   }
+
+   add_wait_queue(sk_sleep(sk), &wait);
+   while (1) {
+   sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+   if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+   goto do_error;
+   if (!*timeo_p) {
+   if (noblock)
+   set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+   goto do_nonblock;
+   }
+   if (signal_pending(current))
+   goto do_interrupted;
+   sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+   if (csk_mem_free(cdev, sk) && !vm_wait)
+   break;
+
+   set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+   sk->sk_write_pending++;
+   sk_wait_event(sk, ¤t_timeo, sk->sk_err ||
+ (sk->sk_shutdown & SEND_SHUTDOWN) ||
+ (csk_mem_free(cdev, sk) && !vm_wait), &wait);
+   sk->sk_write_pending--;
+
+   if (vm_wait) {
+   vm_wait -= current_timeo;
+   current_timeo = *timeo_p;
+   if (current_timeo != MAX_SCHEDULE_TIMEOUT) {
+   current_timeo -= vm_wait;
+   if (current_timeo < 0)
+   current_timeo = 0;
+   }
+   vm_wait = 0;
+   }
+   *timeo_p = current_timeo;
+   }
+do_rm_wq:
+   remove_wait_queue(sk_sleep(sk), &wait);
+   return err;
+do_error:
+   err = -EPIPE;
+   goto do_rm_wq;
+do_nonblock:
+   err = -EAGAIN;
+   goto do_rm_wq;
+do_interrupted:
+   err = sock_intr_errno(*timeo_p);
+   goto do_rm_wq;
+}
+
 int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 {
struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
@@ -952,6 +1024,8 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, 
size_t size)
copy = mss - skb->len;
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
+   if (!csk_mem_free(cdev, sk))
+   goto wait_for_sndbuf;
 
if (is_tls_tx(csk) && !csk->tlshws.txleft) {
struct tls_hdr hdr;
@@ -1099,8 +1173,10 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, 
size_t size)
if (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND)
push_frames_if_head(sk);
continue;
+wait_for_sndbuf

[PATCH v2 3/5] crypto: chtls: dereference null variable

[Atul Gupta]

skb dereferenced before check in sendpage

Reported-by: Dan Carpenter 
Signed-off-by: Atul Gupta 
---
 drivers/crypto/chelsio/chtls/chtls_io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c 
b/drivers/crypto/chelsio/chtls/chtls_io.c
index 7aa5d90..8cfc27b 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -1230,9 +1230,8 @@ int chtls_sendpage(struct sock *sk, struct page *page,
struct sk_buff *skb = skb_peek_tail(&csk->txq);
int copy, i;
 
-   copy = mss - skb->len;
if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||
-   copy <= 0) {
+   (copy = mss - skb->len) <= 0) {
 new_buf:
if (!csk_mem_free(cdev, sk))
goto wait_for_sndbuf;
-- 
1.8.3.1

[PATCH v2 4/5] crypto: chtls: kbuild warnings

[Atul Gupta]

- unindented continue
- check for null page
- signed return

Reported-by: Dan Carpenter 
Signed-off-by: Atul Gupta 
---
 drivers/crypto/chelsio/chtls/chtls_io.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c 
b/drivers/crypto/chelsio/chtls/chtls_io.c
index 8cfc27b..51fc682 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -907,11 +907,11 @@ static int chtls_skb_copy_to_page_nocache(struct sock *sk,
 }
 
 /* Read TLS header to find content type and data length */
-static u16 tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
+static int tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
 {
if (copy_from_iter(thdr, sizeof(*thdr), from) != sizeof(*thdr))
return -EFAULT;
-   return (__force u16)cpu_to_be16(thdr->length);
+   return (__force int)cpu_to_be16(thdr->length);
 }
 
 static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
@@ -1083,9 +1083,10 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, 
size_t size)
int off = TCP_OFF(sk);
bool merge;
 
-   if (page)
-   pg_size <<= compound_order(page);
+   if (!page)
+   goto wait_for_memory;
 
+   pg_size <<= compound_order(page);
if (off < pg_size &&
skb_can_coalesce(skb, i, page, off)) {
merge = 1;
@@ -1492,7 +1493,7 @@ static int chtls_pt_recvmsg(struct sock *sk, struct 
msghdr *msg, size_t len,
break;
chtls_cleanup_rbuf(sk, copied);
sk_wait_data(sk, &timeo, NULL);
-   continue;
+   continue;
 found_ok_skb:
if (!skb->len) {
skb_dst_set(skb, NULL);
-- 
1.8.3.1

Re: PBKDF2 support in the linux kernel

[Theodore Y. Ts'o]

On Sun, May 27, 2018 at 01:22:05PM +0200, Rafael J. Wysocki wrote:
> Again, the PBKDF2 user would be hibernation.  It could either take a key from
> user space, which would require a key-generating user-space component to be
> present in the initramfs (I guess no issue for a regular distro, but I can
> imagine cases when it may be a difficulty), or take a passphrase from user
> space and generate a key by itself (that's what we would like to use PBKDF2
> for).

Right, but are you going to get the passphrase from user space?  You
have to prompt from user space anyway, so running PBPDF2 from
userspace isn't that big of deal.  Feel free to grab the
implementation from e2fsprogs; it's not hard.  :-)

- Ted

Re: PBKDF2 support in the linux kernel

[Rafael J. Wysocki]

On Saturday, May 26, 2018 7:12:36 AM CEST Herbert Xu wrote:
> On Tue, May 22, 2018 at 11:00:40AM +0800, Yu Chen wrote:
> > Hi all,
> > The request is that, we'd like to generate a symmetric key derived from
> > user provided passphase(not rely on any third-party library). May I know if
> > there is a PBKDF2(Password-Based Key Derivation Function 2) support in the
> > kernel? (https://tools.ietf.org/html/rfc2898#5.2)
> > We have hmac sha1 in the kernel, do we have plan to port/implement
> > corresponding PBKDF2 in the kernel too?
> 
> The rule for adding crypto code to the kernel is simple, there
> must be an in-kernel user of the algorithm.

So we are talking about an in-kernel user here.

Again, the PBKDF2 user would be hibernation.  It could either take a key from
user space, which would require a key-generating user-space component to be
present in the initramfs (I guess no issue for a regular distro, but I can
imagine cases when it may be a difficulty), or take a passphrase from user
space and generate a key by itself (that's what we would like to use PBKDF2
for).

We would prefer the latter for various reasons (convenience mostly, but also
not having to rely on user space to do the right thing).

Thanks,
Rafael