Message ID | 20151230034753.GA8776@gondor.apana.org.au (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Herbert Xu |
Headers | show |
Am Mittwoch, 30. Dezember 2015, 11:47:53 schrieb Herbert Xu: Hi Herbert, > On Tue, Dec 29, 2015 at 07:36:14PM +0100, Dmitry Vyukov wrote: > > Hello, > > > > On commit 8513342170278468bac126640a5d2d12ffbff106 > > + crypto: algif_skcipher - Use new skcipher interface > > + crypto: algif_skcipher - Require setkey before accept(2) > > + crypto: af_alg - Disallow bind/setkey/... after accept(2) > > OK there is a silly bug in the last patch. Here is an updated > version. With this patch, the AF_ALG interface stops working. I tested the HMAC operation and I am unable to set the key with the following call: ret = setsockopt(handle->tfmfd, SOL_ALG, ALG_SET_KEY, key, keylen); This call returns EBUSY. The test can be performed with [1] using the following call: test/kcapi -x 3 -c "hmac(sha1)" -k 6e77ebd479da794707bc6cde3694f552ea892dab -p 31b62a797adbff6b8a358d2b5206e01fee079de8cdfc4695138bba163b4efbf30127343e7fd4fbc696c3d38d8f27f57c024b5056f726ceeb4c31d98e57751ec8cbe8904ee0f9b031ae6a0c55da5e062475b3d7832191d4057643ef5fa446801d59a04693e573a8159cd2416b7bd39c7f0fe63c599365e04d596c05736beaab58 Without the patch, all works. [1] http://www.chronox.de/libkcapi.html
Am Freitag, 1. Januar 2016, 21:12:40 schrieb Stephan Mueller: Hi Herbert, > Am Mittwoch, 30. Dezember 2015, 11:47:53 schrieb Herbert Xu: > > Hi Herbert, > > > On Tue, Dec 29, 2015 at 07:36:14PM +0100, Dmitry Vyukov wrote: > > > Hello, > > > > > > On commit 8513342170278468bac126640a5d2d12ffbff106 > > > + crypto: algif_skcipher - Use new skcipher interface > > > + crypto: algif_skcipher - Require setkey before accept(2) > > > + crypto: af_alg - Disallow bind/setkey/... after accept(2) > > > > OK there is a silly bug in the last patch. Here is an updated > > version. > > With this patch, the AF_ALG interface stops working. I tested the HMAC > operation and I am unable to set the key with the following call: > > ret = setsockopt(handle->tfmfd, SOL_ALG, ALG_SET_KEY, key, keylen); > > This call returns EBUSY. Please disregard my email. I did not update my library to the newly requested order of performing the setkey before the accept call. After the update of my library I can confirm that the modification works for all AF_ALG interfaces.
diff --git a/crypto/af_alg.c b/crypto/af_alg.c index a8e7aa3..7b5b592 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -125,6 +125,23 @@ int af_alg_release(struct socket *sock) } EXPORT_SYMBOL_GPL(af_alg_release); +void af_alg_release_parent(struct sock *sk) +{ + struct alg_sock *ask = alg_sk(sk); + bool last; + + sk = ask->parent; + ask = alg_sk(sk); + + lock_sock(sk); + last = !--ask->refcnt; + release_sock(sk); + + if (last) + sock_put(sk); +} +EXPORT_SYMBOL_GPL(af_alg_release_parent); + static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) { const u32 forbidden = CRYPTO_ALG_INTERNAL; @@ -133,6 +150,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) struct sockaddr_alg *sa = (void *)uaddr; const struct af_alg_type *type; void *private; + int err; if (sock->state == SS_CONNECTED) return -EINVAL; @@ -160,16 +178,22 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) return PTR_ERR(private); } + err = -EBUSY; lock_sock(sk); + if (ask->refcnt) + goto unlock; swap(ask->type, type); swap(ask->private, private); + err = 0; + +unlock: release_sock(sk); alg_do_release(type, private); - return 0; + return err; } static int alg_setkey(struct sock *sk, char __user *ukey, @@ -202,11 +226,15 @@ static int alg_setsockopt(struct socket *sock, int level, int optname, struct sock *sk = sock->sk; struct alg_sock *ask = alg_sk(sk); const struct af_alg_type *type; - int err = -ENOPROTOOPT; + int err = -EBUSY; lock_sock(sk); + if (ask->refcnt) + goto unlock; + type = ask->type; + err = -ENOPROTOOPT; if (level != SOL_ALG || !type) goto unlock; @@ -264,7 +292,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock) sk2->sk_family = PF_ALG; - sock_hold(sk); + if (!ask->refcnt++) + sock_hold(sk); alg_sk(sk2)->parent = sk; alg_sk(sk2)->type = type; diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index 018afb2..589716f 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -30,6 +30,8 @@ struct alg_sock { struct sock *parent; + unsigned int refcnt; + const struct af_alg_type *type; void *private; }; @@ -67,6 +69,7 @@ int af_alg_register_type(const struct af_alg_type *type); int af_alg_unregister_type(const struct af_alg_type *type); int af_alg_release(struct socket *sock); +void af_alg_release_parent(struct sock *sk); int af_alg_accept(struct sock *sk, struct socket *newsock); int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len); @@ -83,11 +86,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk) return (struct alg_sock *)sk; } -static inline void af_alg_release_parent(struct sock *sk) -{ - sock_put(alg_sk(sk)->parent); -} - static inline void af_alg_init_completion(struct af_alg_completion *completion) { init_completion(&completion->completion);