| Message ID | tencent_656D589558EA3EED8ACF3C79166F202E010A@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [next] crypto: fix oob Read in arc4_crypt | expand |
On Wed, Dec 20, 2023 at 11:53:55PM +0800, Edward Adam Davis wrote: > The space allocated to areq is not sufficient to access the member __ctx of > struct skcipher_request, as the space occupied by struct arc4_ctx for reading > is 1032 bytes, while the requested memory size in skcipher_recvmsg() is: > sizeof(struct af_alg_async_req) + crypto_skcipher_reqsize(tfm) = 736 bytes, > which does not include the memory required for __ctx of struct skcipher_request. I cannot reproduce this. The total allocated size I get is 1768. How can crypto_skcipher_reqsize(tfm) return zero? Thanks,
On Wed, Dec 20, 2023 at 11:53:55PM +0800, Edward Adam Davis wrote: > The space allocated to areq is not sufficient to access the member __ctx of > struct skcipher_request, as the space occupied by struct arc4_ctx for reading > is 1032 bytes, while the requested memory size in skcipher_recvmsg() is: > sizeof(struct af_alg_async_req) + crypto_skcipher_reqsize(tfm) = 736 bytes, > which does not include the memory required for __ctx of struct skcipher_request. > > Reported-by: syzbot+8ffb0839a24e9c6bfa76@syzkaller.appspotmail.com > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > --- > crypto/algif_skcipher.c | 10 +++++++--- > crypto/skcipher.c | 1 - > include/crypto/internal/skcipher.h | 1 + > 3 files changed, 8 insertions(+), 4 deletions(-) I see where the real bug is. The statesize is not being passed along by ecb so that's why we end up with no memory. Cheers,
Hi Edward, kernel test robot noticed the following build warnings: [auto build test WARNING on next-20231222] url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/crypto-fix-oob-Read-in-arc4_crypt/20231222-172845 base: next-20231222 patch link: https://lore.kernel.org/r/tencent_656D589558EA3EED8ACF3C79166F202E010A%40qq.com patch subject: [PATCH next] crypto: fix oob Read in arc4_crypt config: i386-buildonly-randconfig-003-20231224 (https://download.01.org/0day-ci/archive/20231225/202312250259.yyBgM27K-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231225/202312250259.yyBgM27K-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312250259.yyBgM27K-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/crypto/padlock-aes.c:13: >> include/crypto/internal/skcipher.h:27:33: warning: 'crypto_skcipher_type' defined but not used [-Wunused-const-variable=] 27 | static const struct crypto_type crypto_skcipher_type; | ^~~~~~~~~~~~~~~~~~~~ vim +/crypto_skcipher_type +27 include/crypto/internal/skcipher.h 24 25 struct aead_request; 26 struct rtattr; > 27 static const struct crypto_type crypto_skcipher_type; 28
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 02cea2149504..b69d361b5515 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -33,6 +33,7 @@ #include <linux/module.h> #include <linux/net.h> #include <net/sock.h> +#include <crypto/internal/skcipher.h> static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) @@ -102,11 +103,12 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, struct alg_sock *pask = alg_sk(psk); struct af_alg_ctx *ctx = ask->private; struct crypto_skcipher *tfm = pask->private; + struct skcipher_alg *alg = crypto_skcipher_alg(tfm); unsigned int bs = crypto_skcipher_chunksize(tfm); struct af_alg_async_req *areq; unsigned cflags = 0; int err = 0; - size_t len = 0; + size_t len = 0, aqlen; if (!ctx->init || (ctx->more && ctx->used < bs)) { err = af_alg_wait_for_data(sk, flags, bs); @@ -115,8 +117,10 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, } /* Allocate cipher request for current operation. */ - areq = af_alg_alloc_areq(sk, sizeof(struct af_alg_async_req) + - crypto_skcipher_reqsize(tfm)); + aqlen = sizeof(struct af_alg_async_req) + crypto_skcipher_reqsize(tfm); + if (alg->co.base.cra_type != &crypto_skcipher_type) + aqlen += 1032; + areq = af_alg_alloc_areq(sk, aqlen); if (IS_ERR(areq)) return PTR_ERR(areq); diff --git a/crypto/skcipher.c b/crypto/skcipher.c index bc70e159d27d..0ae4a05a5aa7 100644 --- a/crypto/skcipher.c +++ b/crypto/skcipher.c @@ -44,7 +44,6 @@ struct skcipher_walk_buffer { u8 buffer[]; }; -static const struct crypto_type crypto_skcipher_type; static int skcipher_walk_next(struct skcipher_walk *walk); diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h index 7ae42afdcf3e..3c05872652f2 100644 --- a/include/crypto/internal/skcipher.h +++ b/include/crypto/internal/skcipher.h @@ -24,6 +24,7 @@ struct aead_request; struct rtattr; +static const struct crypto_type crypto_skcipher_type; struct skcipher_instance { void (*free)(struct skcipher_instance *inst);
The space allocated to areq is not sufficient to access the member __ctx of struct skcipher_request, as the space occupied by struct arc4_ctx for reading is 1032 bytes, while the requested memory size in skcipher_recvmsg() is: sizeof(struct af_alg_async_req) + crypto_skcipher_reqsize(tfm) = 736 bytes, which does not include the memory required for __ctx of struct skcipher_request. Reported-by: syzbot+8ffb0839a24e9c6bfa76@syzkaller.appspotmail.com Signed-off-by: Edward Adam Davis <eadavis@qq.com> --- crypto/algif_skcipher.c | 10 +++++++--- crypto/skcipher.c | 1 - include/crypto/internal/skcipher.h | 1 + 3 files changed, 8 insertions(+), 4 deletions(-)