| Message ID | 20211229093216.1753083-1-jiasheng@iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: af_alg - check possible NULL pointer | expand |
On Wed, Dec 29, 2021 at 05:32:16PM +0800, Jiasheng Jiang wrote: > And then it will cause the BUG_ON() in sg_assign_page(). That's not true. The two BUG_ON()'s in sg_assign_page() are: BUG_ON((unsigned long) page & (SG_CHAIN | SG_END)); #ifdef CONFIG_DEBUG_SG BUG_ON(sg_is_chain(sg)); #endif Neither will trigger due to page == NULL. > do { > + struct page *pg; > unsigned int i = sgl->cur; > > plen = min_t(size_t, len, PAGE_SIZE); > > - sg_assign_page(sg + i, alloc_page(GFP_KERNEL)); > + pg = alloc_page(GFP_KERNEL); > + if (!pg) { > + err = -ENOMEM; > + goto unlock; > + } > + > + sg_assign_page(sg + i, pg); > if (!sg_page(sg + i)) { > err = -ENOMEM; > goto unlock; The NULL check is already done, just below the redundant one you're adding. I do think it would be more logical to do the NULL check before sg_assign_page(). So you could send a patch that moves it there. But it would be a cleanup, not a fix. - Eric
diff --git a/crypto/af_alg.c b/crypto/af_alg.c index 18cc82dc4a42..a1c0118e222d 100644 --- a/crypto/af_alg.c +++ b/crypto/af_alg.c @@ -931,11 +931,18 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size, sg_unmark_end(sg + sgl->cur - 1); do { + struct page *pg; unsigned int i = sgl->cur; plen = min_t(size_t, len, PAGE_SIZE); - sg_assign_page(sg + i, alloc_page(GFP_KERNEL)); + pg = alloc_page(GFP_KERNEL); + if (!pg) { + err = -ENOMEM; + goto unlock; + } + + sg_assign_page(sg + i, pg); if (!sg_page(sg + i)) { err = -ENOMEM; goto unlock;
Because of the possible alloc failure of the alloc_page(), it could return NULL pointer. And then it will cause the BUG_ON() in sg_assign_page(). Therefore, it should be better to check it before to avoid the bug. Fixes: 2d97591ef43d ("crypto: af_alg - consolidation of duplicate code") Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> --- crypto/af_alg.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)