| Message ID | 53de0ccd1aa3fffa6bce2a2ae7a5ca07e0af6d3a.1625900431.git.paskripkin@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: cipso: bug fixes | expand |
On Sat, Jul 10, 2021 at 3:03 AM Pavel Skripkin <paskripkin@gmail.com> wrote: > > Syzbot reported warning in netlbl_cipsov4_add(). The > problem was in too big doi_def->map.std->lvl.local_size > passed to kcalloc(). Since this value comes from userpace there is > no need to warn if value is not correct. > > The same problem may occur with other kcalloc() calls in > this function, so, I've added __GFP_NOWARN flag to all > kcalloc() calls there. > > Reported-and-tested-by: syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com > Fixes: 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration") > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > --- > net/netlabel/netlabel_cipso_v4.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) This seems fine to me, callers will get a ENOMEM error code too so it isn't like the failure is going to be a mystery, especially in the case where an obscenely large translation mapping is being attempted. Acked-by: Paul Moore <paul@paul-moore.com> As an aside, I see no reason why this patch can't be merged and 2/2 simply dropped as already in-tree. As has already been pointed out, patch 2/2 is a duplicate; the in-tree commit is d612c3f3fae2 ("net: ipv4: fix memory leak in netlbl_cipsov4_add_std"). > diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c > index 4f50a64315cf..50f40943c815 100644 > --- a/net/netlabel/netlabel_cipso_v4.c > +++ b/net/netlabel/netlabel_cipso_v4.c > @@ -187,14 +187,14 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, > } > doi_def->map.std->lvl.local = kcalloc(doi_def->map.std->lvl.local_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | __GFP_NOWARN); > if (doi_def->map.std->lvl.local == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > } > doi_def->map.std->lvl.cipso = kcalloc(doi_def->map.std->lvl.cipso_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | __GFP_NOWARN); > if (doi_def->map.std->lvl.cipso == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > @@ -263,7 +263,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, > doi_def->map.std->cat.local = kcalloc( > doi_def->map.std->cat.local_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | __GFP_NOWARN); > if (doi_def->map.std->cat.local == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > @@ -271,7 +271,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, > doi_def->map.std->cat.cipso = kcalloc( > doi_def->map.std->cat.cipso_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | __GFP_NOWARN); > if (doi_def->map.std->cat.cipso == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > -- > 2.32.0 -- paul moore www.paul-moore.com
On Sat, 10 Jul 2021 10:03:13 +0300 Pavel Skripkin <paskripkin@gmail.com> wrote: > Syzbot reported warning in netlbl_cipsov4_add(). The > problem was in too big doi_def->map.std->lvl.local_size > passed to kcalloc(). Since this value comes from userpace there is > no need to warn if value is not correct. > > The same problem may occur with other kcalloc() calls in > this function, so, I've added __GFP_NOWARN flag to all > kcalloc() calls there. > > Reported-and-tested-by: > syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com Fixes: > 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration") > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- > net/netlabel/netlabel_cipso_v4.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/netlabel/netlabel_cipso_v4.c > b/net/netlabel/netlabel_cipso_v4.c index 4f50a64315cf..50f40943c815 > 100644 --- a/net/netlabel/netlabel_cipso_v4.c > +++ b/net/netlabel/netlabel_cipso_v4.c > @@ -187,14 +187,14 @@ static int netlbl_cipsov4_add_std(struct > genl_info *info, } > doi_def->map.std->lvl.local = > kcalloc(doi_def->map.std->lvl.local_size, sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | > __GFP_NOWARN); if (doi_def->map.std->lvl.local == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > } > doi_def->map.std->lvl.cipso = > kcalloc(doi_def->map.std->lvl.cipso_size, sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | > __GFP_NOWARN); if (doi_def->map.std->lvl.cipso == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > @@ -263,7 +263,7 @@ static int netlbl_cipsov4_add_std(struct > genl_info *info, doi_def->map.std->cat.local = kcalloc( > doi_def->map.std->cat.local_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | > __GFP_NOWARN); if (doi_def->map.std->cat.local == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > @@ -271,7 +271,7 @@ static int netlbl_cipsov4_add_std(struct > genl_info *info, doi_def->map.std->cat.cipso = kcalloc( > doi_def->map.std->cat.cipso_size, > sizeof(u32), > - GFP_KERNEL); > + GFP_KERNEL | > __GFP_NOWARN); if (doi_def->map.std->cat.cipso == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; Hi, net developers! Is this patch merged somewhere? I've checked net tree and Paul Moore tree on https://git.kernel.org/, but didn't find it. Did I miss it somewhere? If not, it's just a gentle ping :) Btw: maybe I should send it as separete patch, since 2/2 in this series is invalid as already in-tree? With regards, Pavel Skripkin
On Mon, Jul 26, 2021 at 7:11 AM Pavel Skripkin <paskripkin@gmail.com> wrote: > On Sat, 10 Jul 2021 10:03:13 +0300 > Pavel Skripkin <paskripkin@gmail.com> wrote: > > > Syzbot reported warning in netlbl_cipsov4_add(). The > > problem was in too big doi_def->map.std->lvl.local_size > > passed to kcalloc(). Since this value comes from userpace there is > > no need to warn if value is not correct. > > > > The same problem may occur with other kcalloc() calls in > > this function, so, I've added __GFP_NOWARN flag to all > > kcalloc() calls there. > > > > Reported-and-tested-by: > > syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com Fixes: > > 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration") > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- > > net/netlabel/netlabel_cipso_v4.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > Hi, net developers! > > Is this patch merged somewhere? I've checked net tree and Paul Moore > tree on https://git.kernel.org/, but didn't find it. Did I miss it > somewhere? If not, it's just a gentle ping :) > > Btw: maybe I should send it as separete patch, since 2/2 in this > series is invalid as already in-tree? I'm not sure why this hasn't been picked up yet, but I suppose resubmitting just this patch couldn't hurt. Don't forget to include my ACK if you do. -- paul moore www.paul-moore.com
diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c index 4f50a64315cf..50f40943c815 100644 --- a/net/netlabel/netlabel_cipso_v4.c +++ b/net/netlabel/netlabel_cipso_v4.c @@ -187,14 +187,14 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, } doi_def->map.std->lvl.local = kcalloc(doi_def->map.std->lvl.local_size, sizeof(u32), - GFP_KERNEL); + GFP_KERNEL | __GFP_NOWARN); if (doi_def->map.std->lvl.local == NULL) { ret_val = -ENOMEM; goto add_std_failure; } doi_def->map.std->lvl.cipso = kcalloc(doi_def->map.std->lvl.cipso_size, sizeof(u32), - GFP_KERNEL); + GFP_KERNEL | __GFP_NOWARN); if (doi_def->map.std->lvl.cipso == NULL) { ret_val = -ENOMEM; goto add_std_failure; @@ -263,7 +263,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, doi_def->map.std->cat.local = kcalloc( doi_def->map.std->cat.local_size, sizeof(u32), - GFP_KERNEL); + GFP_KERNEL | __GFP_NOWARN); if (doi_def->map.std->cat.local == NULL) { ret_val = -ENOMEM; goto add_std_failure; @@ -271,7 +271,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info, doi_def->map.std->cat.cipso = kcalloc( doi_def->map.std->cat.cipso_size, sizeof(u32), - GFP_KERNEL); + GFP_KERNEL | __GFP_NOWARN); if (doi_def->map.std->cat.cipso == NULL) { ret_val = -ENOMEM; goto add_std_failure;
Syzbot reported warning in netlbl_cipsov4_add(). The problem was in too big doi_def->map.std->lvl.local_size passed to kcalloc(). Since this value comes from userpace there is no need to warn if value is not correct. The same problem may occur with other kcalloc() calls in this function, so, I've added __GFP_NOWARN flag to all kcalloc() calls there. Reported-and-tested-by: syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com Fixes: 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration") Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- net/netlabel/netlabel_cipso_v4.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)