Re: [crypto 6/8] chtls: TCB and Key program
Am Donnerstag, 7. Dezember 2017, 16:08:04 CET schrieb Atul Gupta: Hi Atul, > > As far as I see, the key is part of the skb (via kctx). This skb is > > released after being processed. The release calls kfree_skb which does > > not zeroize the key. Wouldn't it make sense to clear the memory of the > > key when the skb is released? [Atul] we should perhaps memset the info > > received from user so that driver has no info on key once its written on > > chip memory. memset(gcm_ctx->key, 0, keylen); > > Are you saying that the skb (via kctx) above does not obtain a copy of the > key? If not, what is done in chtls_key_info? It does have a key copy, I was > not sure how key info is accessed once skb is released. All I am saying is that this key copy should be zapped when the memory is released. Thus, if the skb has a copy of the key, at least at the time of free() of the skb, a memset() of the key memory should be done (or before). > > > Ciao > Stephan > > Thanks > Atul Ciao Stephan
RE: [crypto 6/8] chtls: TCB and Key program
-Original Message- From: linux-crypto-ow...@vger.kernel.org [mailto:linux-crypto-ow...@vger.kernel.org] On Behalf Of Stephan Mueller Sent: Thursday, December 7, 2017 8:13 PM To: Atul Gupta Cc: herb...@gondor.apana.org.au; linux-crypto@vger.kernel.org; net...@vger.kernel.org; da...@davemloft.net; davejwat...@fb.com; Ganesh GR ; Harsh Jain Subject: Re: [crypto 6/8] chtls: TCB and Key program Am Donnerstag, 7. Dezember 2017, 15:21:03 CET schrieb Atul Gupta: Hi Atul, > > memzero_explicit(key)? > [Atul] may not be required as entire info of size keylen and > AEAD_H_SIZE is copied onto kctx->key. Key data is received from user, > while ghash is memset and locally generated Sure, but wouldn't it make sense to zap all instances where key material was stored? Agree, Its safe to memset where keylen is variable, perhaps in future where we support different keylen. In current case key len is same as buffer size hence may not cause issue. > > As far as I see, the key is part of the skb (via kctx). This skb is > released after being processed. The release calls kfree_skb which does > not zeroize the key. Wouldn't it make sense to clear the memory of the > key when the skb is released? [Atul] we should perhaps memset the info > received from user so that driver has no info on key once its written on chip > memory. > memset(gcm_ctx->key, 0, keylen); Are you saying that the skb (via kctx) above does not obtain a copy of the key? If not, what is done in chtls_key_info? It does have a key copy, I was not sure how key info is accessed once skb is released. Ciao Stephan Thanks Atul
Re: [crypto 6/8] chtls: TCB and Key program
Am Donnerstag, 7. Dezember 2017, 15:21:03 CET schrieb Atul Gupta: Hi Atul, > > memzero_explicit(key)? > [Atul] may not be required as entire info of size keylen and AEAD_H_SIZE is > copied onto kctx->key. Key data is received from user, while ghash is > memset and locally generated Sure, but wouldn't it make sense to zap all instances where key material was stored? > > As far as I see, the key is part of the skb (via kctx). This skb is released > after being processed. The release calls kfree_skb which does not zeroize > the key. Wouldn't it make sense to clear the memory of the key when the skb > is released? [Atul] we should perhaps memset the info received from user so > that driver has no info on key once its written on chip memory. > memset(gcm_ctx->key, 0, keylen); Are you saying that the skb (via kctx) above does not obtain a copy of the key? If not, what is done in chtls_key_info? Ciao Stephan
RE: [crypto 6/8] chtls: TCB and Key program
-Original Message- From: Stephan Mueller [mailto:smuel...@chronox.de] Sent: Tuesday, December 5, 2017 6:37 PM To: Atul Gupta Cc: herb...@gondor.apana.org.au; linux-crypto@vger.kernel.org; net...@vger.kernel.org; da...@davemloft.net; davejwat...@fb.com; Ganesh GR ; Harsh Jain Subject: Re: [crypto 6/8] chtls: TCB and Key program Am Dienstag, 5. Dezember 2017, 12:40:29 CET schrieb Atul Gupta: Hi Atul, > program the tx and rx key on chip. > > Signed-off-by: Atul Gupta <mailto:atul.gu...@chelsio.com> > --- > drivers/crypto/chelsio/chtls/chtls_hw.c | 394 > 1 file changed, 394 insertions(+) > create mode 100644 drivers/crypto/chelsio/chtls/chtls_hw.c > > diff --git a/drivers/crypto/chelsio/chtls/chtls_hw.c > b/drivers/crypto/chelsio/chtls/chtls_hw.c new file mode 100644 index > 000..5e65aa2 > --- /dev/null > +++ b/drivers/crypto/chelsio/chtls/chtls_hw.c > @@ -0,0 +1,394 @@ > +/* > + * Copyright (c) 2017 Chelsio Communications, Inc. > + * > + * This program is free software; you can redistribute it and/or > +modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Written by: Atul Gupta (mailto:atul.gu...@chelsio.com) */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "chtls.h" > +#include "chtls_cm.h" > + > +static void __set_tcb_field_direct(struct chtls_sock *csk, > +struct cpl_set_tcb_field *req, u16 word, > +u64 mask, u64 val, u8 cookie, int no_reply) { > + struct ulptx_idata *sc; > + > + INIT_TP_WR_CPL(req, CPL_SET_TCB_FIELD, csk->tid); > + req->wr.wr_mid |= htonl(FW_WR_FLOWID_V(csk->tid)); > + req->reply_ctrl = htons(NO_REPLY_V(no_reply) | > + QUEUENO_V(csk->rss_qid)); > + req->word_cookie = htons(TCB_WORD(word) | TCB_COOKIE_V(cookie)); > + req->mask = cpu_to_be64(mask); > + req->val = cpu_to_be64(val); > + sc = (struct ulptx_idata *)(req + 1); > + sc->cmd_more = htonl(ULPTX_CMD_V(ULP_TX_SC_NOOP)); > + sc->len = htonl(0); > +} > + > +void __set_tcb_field(struct sock *sk, struct sk_buff *skb, u16 word, > + u64 mask, u64 val, u8 cookie, int no_reply) { > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + > + req = (struct cpl_set_tcb_field *)__skb_put(skb, wrlen); > + __set_tcb_field_direct(csk, req, word, mask, val, cookie, no_reply); > + set_wr_txq(skb, CPL_PRIORITY_CONTROL, csk->port_id); } > + > +static int chtls_set_tcb_field(struct sock *sk, u16 word, u64 mask, > +u64 > val) +{ > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct sk_buff *skb; > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + unsigned int credits_needed = DIV_ROUND_UP(wrlen, 16); > + > + skb = alloc_skb(wrlen, GFP_ATOMIC); > + if (!skb) > + return -ENOMEM; > + > + __set_tcb_field(sk, skb, word, mask, val, 0, 1); > + set_queue(skb, (csk->txq_idx << 1) | CPL_PRIORITY_DATA, sk); > + csk->wr_credits -= credits_needed; > + csk->wr_unacked += credits_needed; > + enqueue_wr(csk, skb); > + cxgb4_ofld_send(csk->egress_dev, skb); > + return 0; > +} > + > +/* > + * Set one of the t_flags bits in the TCB. > + */ > +int chtls_set_tcb_tflag(struct sock *sk, unsigned int bit_pos, int > +val) { > + return chtls_set_tcb_field(sk, 1, 1ULL << bit_pos, > + val << bit_pos); > +} > + > +static int chtls_set_tcb_keyid(struct sock *sk, int keyid) { > + return chtls_set_tcb_field(sk, 31, 0xULL, keyid); } > + > +static int chtls_set_tcb_seqno(struct sock *sk) { > + return chtls_set_tcb_field(sk, 28, ~0ULL, 0); } > + > +static int chtls_set_tcb_quiesce(struct sock *sk, int val) { > + return chtls_set_tcb_field(sk, 1, (1ULL << TF_RX_QUIESCE_S), > +TF_RX_QUIESCE_V(val)); > +} > + > +static void *chtls_alloc_mem(unsigned long size) { > + void *p = kmalloc(size, GFP_KERNEL); > + > + if (!p) > + p = vmalloc(size); > + if (p) > + memset(p, 0, size); > +
Re: [crypto 6/8] chtls: TCB and Key program
Am Dienstag, 5. Dezember 2017, 12:40:29 CET schrieb Atul Gupta: Hi Atul, > program the tx and rx key on chip. > > Signed-off-by: Atul Gupta > --- > drivers/crypto/chelsio/chtls/chtls_hw.c | 394 > 1 file changed, 394 insertions(+) > create mode 100644 drivers/crypto/chelsio/chtls/chtls_hw.c > > diff --git a/drivers/crypto/chelsio/chtls/chtls_hw.c > b/drivers/crypto/chelsio/chtls/chtls_hw.c new file mode 100644 > index 000..5e65aa2 > --- /dev/null > +++ b/drivers/crypto/chelsio/chtls/chtls_hw.c > @@ -0,0 +1,394 @@ > +/* > + * Copyright (c) 2017 Chelsio Communications, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Written by: Atul Gupta (atul.gu...@chelsio.com) > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "chtls.h" > +#include "chtls_cm.h" > + > +static void __set_tcb_field_direct(struct chtls_sock *csk, > +struct cpl_set_tcb_field *req, u16 word, > +u64 mask, u64 val, u8 cookie, int no_reply) > +{ > + struct ulptx_idata *sc; > + > + INIT_TP_WR_CPL(req, CPL_SET_TCB_FIELD, csk->tid); > + req->wr.wr_mid |= htonl(FW_WR_FLOWID_V(csk->tid)); > + req->reply_ctrl = htons(NO_REPLY_V(no_reply) | > + QUEUENO_V(csk->rss_qid)); > + req->word_cookie = htons(TCB_WORD(word) | TCB_COOKIE_V(cookie)); > + req->mask = cpu_to_be64(mask); > + req->val = cpu_to_be64(val); > + sc = (struct ulptx_idata *)(req + 1); > + sc->cmd_more = htonl(ULPTX_CMD_V(ULP_TX_SC_NOOP)); > + sc->len = htonl(0); > +} > + > +void __set_tcb_field(struct sock *sk, struct sk_buff *skb, u16 word, > + u64 mask, u64 val, u8 cookie, int no_reply) > +{ > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + > + req = (struct cpl_set_tcb_field *)__skb_put(skb, wrlen); > + __set_tcb_field_direct(csk, req, word, mask, val, cookie, no_reply); > + set_wr_txq(skb, CPL_PRIORITY_CONTROL, csk->port_id); > +} > + > +static int chtls_set_tcb_field(struct sock *sk, u16 word, u64 mask, u64 > val) +{ > + struct chtls_sock *csk = rcu_dereference_sk_user_data(sk); > + struct sk_buff *skb; > + struct cpl_set_tcb_field *req; > + struct ulptx_idata *sc; > + unsigned int wrlen = roundup(sizeof(*req) + sizeof(*sc), 16); > + unsigned int credits_needed = DIV_ROUND_UP(wrlen, 16); > + > + skb = alloc_skb(wrlen, GFP_ATOMIC); > + if (!skb) > + return -ENOMEM; > + > + __set_tcb_field(sk, skb, word, mask, val, 0, 1); > + set_queue(skb, (csk->txq_idx << 1) | CPL_PRIORITY_DATA, sk); > + csk->wr_credits -= credits_needed; > + csk->wr_unacked += credits_needed; > + enqueue_wr(csk, skb); > + cxgb4_ofld_send(csk->egress_dev, skb); > + return 0; > +} > + > +/* > + * Set one of the t_flags bits in the TCB. > + */ > +int chtls_set_tcb_tflag(struct sock *sk, unsigned int bit_pos, int val) > +{ > + return chtls_set_tcb_field(sk, 1, 1ULL << bit_pos, > + val << bit_pos); > +} > + > +static int chtls_set_tcb_keyid(struct sock *sk, int keyid) > +{ > + return chtls_set_tcb_field(sk, 31, 0xULL, keyid); > +} > + > +static int chtls_set_tcb_seqno(struct sock *sk) > +{ > + return chtls_set_tcb_field(sk, 28, ~0ULL, 0); > +} > + > +static int chtls_set_tcb_quiesce(struct sock *sk, int val) > +{ > + return chtls_set_tcb_field(sk, 1, (1ULL << TF_RX_QUIESCE_S), > +TF_RX_QUIESCE_V(val)); > +} > + > +static void *chtls_alloc_mem(unsigned long size) > +{ > + void *p = kmalloc(size, GFP_KERNEL); > + > + if (!p) > + p = vmalloc(size); > + if (p) > + memset(p, 0, size); > + return p; > +} > + > +static void chtls_free_mem(void *addr) > +{ > + unsigned long p = (unsigned long)addr; > + > + if (p >= VMALLOC_START && p < VMALLOC_END) > + vfree(addr); > + else > + kfree(addr); > +} > + > +/* TLS Key bitmap processing */ > +int chtls_init_kmap(struct chtls_dev *cdev, struct cxgb4_lld_info *lldi) > +{ > + unsigned int num_key_ctx, bsize; > + > + num_key_ctx = (lldi->vr->key.size / TLS_KEY_CONTEXT_SZ); > + bsize = BITS_TO_LONGS(num_key_ctx); > + > + cdev->kmap.size = num_key_ctx; > + cdev->kmap.available = bsize; > + cdev->kmap.addr = chtls_alloc_mem(sizeof(*cdev->kmap.addr) * > + bsize); > + if (!cdev->kmap.addr) > + return -1; > + > + cdev->kmap.start = lldi->vr->key.start;