| Message ID | 2380853.MJYDG7HQLD@positron.chronox.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Am Samstag, 1. April 2017, 17:04:28 CEST schrieb Stephan Müller: Hi Herbert, > Hi Herbert, > > If you concur with the patch, I think it should go to 4.11 as well as > to stable. After checking this issue again, I see that it is not triggerable in the current code as the different af_alg users make sure that this function is not called with 0. I only triggered this issue during experimenting with the algif_skcipher and algif_aead revamp as requested by you. During those experiments, I invoked af_alg_make_sg with a len = 0. Thus, this patch is not applicable for stable and 4.11. Yet, I would suggest to consider this patch as a safeguard for any potential programming errors. Ciao Stephan
On Sat, Apr 01, 2017 at 07:46:39PM +0200, Stephan Müller wrote: > Am Samstag, 1. April 2017, 17:04:28 CEST schrieb Stephan Müller: > > Hi Herbert, > > > Hi Herbert, > > > > If you concur with the patch, I think it should go to 4.11 as well as > > to stable. > > After checking this issue again, I see that it is not triggerable in the > current code as the different af_alg users make sure that this function is not > called with 0. > > I only triggered this issue during experimenting with the algif_skcipher and > algif_aead revamp as requested by you. During those experiments, I invoked > af_alg_make_sg with a len = 0. > > Thus, this patch is not applicable for stable and 4.11. > > Yet, I would suggest to consider this patch as a safeguard for any potential > programming errors. So this is only possible with patches that have not been applied yet? In that case please fold this into whichever patch series that needs it. Thanks,
Am Mittwoch, 5. April 2017, 14:50:12 CEST schrieb Herbert Xu: Hi Herbert, > So this is only possible with patches that have not been applied > yet? Correct. > In that case please fold this into whichever patch series > that needs it. Ok, let us defer it. Ciao Stephan
diff --git a/crypto/af_alg.c b/crypto/af_alg.c index 24dc082..5992997 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -399,12 +399,20 @@ static const struct net_proto_family alg_family = { .owner = THIS_MODULE, }; -int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len) +int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, size_t len) { size_t off; ssize_t n; int npages, i; + if (!len) { + /* init one for linking */ + sg_init_table(sgl->sg, 1); + sg_mark_end(sgl->sg); + sgl->npages = 0; + return 0; + } + n = iov_iter_get_pages(iter, sgl->pages, len, ALG_MAX_PAGES, &off); if (n < 0) return n; diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index 6c3e6e7..c637ac9 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -76,7 +76,7 @@ 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); +int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, size_t len); void af_alg_free_sg(struct af_alg_sgl *sgl); void af_alg_link_sg(struct af_alg_sgl *sgl_prev, struct af_alg_sgl *sgl_new);
Hi Herbert, If you concur with the patch, I think it should go to 4.11 as well as to stable. Ciao Stephan ---8<--- The function af_alg_make_sg converts user-provided IOVECs into an SGL. Thus it operates directly on the user-space provided number of IOVECs. When user space provides 0 for the number of IOVECs iov_iter_get_pages returns a bogus number of bytes. This in turn will cause a crash when the SGL is processed. The fix initializes an SGL with one entry for handling chaining operation but does not contain data. In addition, the patch changes variable type of len from int to size_t to be consistent with the data type of the invoker and the data type where len is used. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- crypto/af_alg.c | 10 +++++++++- include/crypto/if_alg.h | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-)