| Message ID | 1478170170-4748-1-git-send-email-tang.junhui@zte.com.cn (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
On Thu, Nov 03 2016 at 6:49am -0400, tang.junhui@zte.com.cn <tang.junhui@zte.com.cn> wrote: > From: "tang.junhui" <tang.junhui@zte.com.cn> > > pg is not assigned to a group address when count of multipath groups > is zero in bypass_pg_num(), then it is used in bypass_pg(), which may > cause wild pointer access. > > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn> > --- > drivers/md/dm-mpath.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c > index d376dc8..8c1359c 100644 > --- a/drivers/md/dm-mpath.c > +++ b/drivers/md/dm-mpath.c > @@ -1084,7 +1084,7 @@ static int switch_pg_num(struct multipath *m, const char *pgstr) > char dummy; > > if (!pgstr || (sscanf(pgstr, "%u%c", &pgnum, &dummy) != 1) || !pgnum || > - (pgnum > m->nr_priority_groups)) { > + !m->nr_priority_groups || (pgnum > m->nr_priority_groups)) { > DMWARN("invalid PG number supplied to switch_pg_num"); > return -EINVAL; > } > -- > 2.8.1.windows.1 > > You mention bypass_pg_num() going on to hit a NULL/"wild" pointer. Not immediately seeing the relation between switch_pg_num() and bypass_pg_num(). But shouldn't bypass_pg_num() have improved bounds checking (and/or NULL pointer checks) too? Maybe your patch was applied with an offset and it modified switch_pg_num() when you really meant to modify bypass_pg_num()? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hello Mike, I'm sorry to send you the wrong patch, I'll send a new patch to you later. Thanks Tang 发件人: Mike Snitzer <snitzer@redhat.com> 收件人: tang.junhui@zte.com.cn, 抄送: zhang.kai16@zte.com.cn, dm-devel@redhat.com, agk@redhat.com 日期: 2016/11/03 23:33 主题: Re: [dm-devel] dm mpath: add check for count of groups to avoid wild pointer access 发件人: dm-devel-bounces@redhat.com On Thu, Nov 03 2016 at 6:49am -0400, tang.junhui@zte.com.cn <tang.junhui@zte.com.cn> wrote: > From: "tang.junhui" <tang.junhui@zte.com.cn> > > pg is not assigned to a group address when count of multipath groups > is zero in bypass_pg_num(), then it is used in bypass_pg(), which may > cause wild pointer access. > > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn> > --- > drivers/md/dm-mpath.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c > index d376dc8..8c1359c 100644 > --- a/drivers/md/dm-mpath.c > +++ b/drivers/md/dm-mpath.c > @@ -1084,7 +1084,7 @@ static int switch_pg_num(struct multipath *m, const char *pgstr) > char dummy; > > if (!pgstr || (sscanf(pgstr, "%u%c", &pgnum, &dummy) != 1) || !pgnum || > - (pgnum > m->nr_priority_groups)) { > + !m->nr_priority_groups || (pgnum > m->nr_priority_groups)) { > DMWARN("invalid PG number supplied to switch_pg_num"); > return -EINVAL; > } > -- > 2.8.1.windows.1 > > You mention bypass_pg_num() going on to hit a NULL/"wild" pointer. Not immediately seeing the relation between switch_pg_num() and bypass_pg_num(). But shouldn't bypass_pg_num() have improved bounds checking (and/or NULL pointer checks) too? Maybe your patch was applied with an offset and it modified switch_pg_num() when you really meant to modify bypass_pg_num()? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index d376dc8..8c1359c 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -1084,7 +1084,7 @@ static int switch_pg_num(struct multipath *m, const char *pgstr) char dummy; if (!pgstr || (sscanf(pgstr, "%u%c", &pgnum, &dummy) != 1) || !pgnum || - (pgnum > m->nr_priority_groups)) { + !m->nr_priority_groups || (pgnum > m->nr_priority_groups)) { DMWARN("invalid PG number supplied to switch_pg_num"); return -EINVAL; }