| Message ID | 1560516085-3101-1-git-send-email-92siuyang@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/hotplug: fix potential null pointer deference | expand |
On Fri, Jun 14, 2019 at 3:40 PM Young Xiao <92siuyang@gmail.com> wrote: > > There is otherwise a risk of a null pointer dereference. Had you analyze the code? How come that prevnode can become NULL? > Signed-off-by: Young Xiao <92siuyang@gmail.com> > --- > drivers/pci/hotplug/cpqphp_ctrl.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c > index b7f4e1f..3c8399f 100644 > --- a/drivers/pci/hotplug/cpqphp_ctrl.c > +++ b/drivers/pci/hotplug/cpqphp_ctrl.c > @@ -598,10 +598,11 @@ static struct pci_resource *get_io_resource(struct pci_resource **head, u32 size > *head = node->next; > } else { > prevnode = *head; > - while (prevnode->next != node) > + while (prevnode && prevnode->next != node) > prevnode = prevnode->next; > > - prevnode->next = node->next; > + if (prevnode) > + prevnode->next = node->next; > } > node->next = NULL; > break; > @@ -788,10 +789,11 @@ static struct pci_resource *get_resource(struct pci_resource **head, u32 size) > *head = node->next; > } else { > prevnode = *head; > - while (prevnode->next != node) > + while (prevnode && prevnode->next != node) > prevnode = prevnode->next; > > - prevnode->next = node->next; > + if (prevnode) > + prevnode->next = node->next; > } > node->next = NULL; > break; > -- > 2.7.4 >
On Fri, Jun 14, 2019 at 04:56:32PM +0300, Andy Shevchenko wrote: > On Fri, Jun 14, 2019 at 3:40 PM Young Xiao <92siuyang@gmail.com> wrote: > > > > There is otherwise a risk of a null pointer dereference. > > Had you analyze the code? > How come that prevnode can become NULL? Dropping this for now. Young, if there's a scenario where prevnode can actually be NULL, please mention that in the changelog and repost this. > > Signed-off-by: Young Xiao <92siuyang@gmail.com> > > --- > > drivers/pci/hotplug/cpqphp_ctrl.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c > > index b7f4e1f..3c8399f 100644 > > --- a/drivers/pci/hotplug/cpqphp_ctrl.c > > +++ b/drivers/pci/hotplug/cpqphp_ctrl.c > > @@ -598,10 +598,11 @@ static struct pci_resource *get_io_resource(struct pci_resource **head, u32 size > > *head = node->next; > > } else { > > prevnode = *head; > > - while (prevnode->next != node) > > + while (prevnode && prevnode->next != node) > > prevnode = prevnode->next; > > > > - prevnode->next = node->next; > > + if (prevnode) > > + prevnode->next = node->next; > > } > > node->next = NULL; > > break; > > @@ -788,10 +789,11 @@ static struct pci_resource *get_resource(struct pci_resource **head, u32 size) > > *head = node->next; > > } else { > > prevnode = *head; > > - while (prevnode->next != node) > > + while (prevnode && prevnode->next != node) > > prevnode = prevnode->next; > > > > - prevnode->next = node->next; > > + if (prevnode) > > + prevnode->next = node->next; > > } > > node->next = NULL; > > break; > > -- > > 2.7.4 > > > > > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c index b7f4e1f..3c8399f 100644 --- a/drivers/pci/hotplug/cpqphp_ctrl.c +++ b/drivers/pci/hotplug/cpqphp_ctrl.c @@ -598,10 +598,11 @@ static struct pci_resource *get_io_resource(struct pci_resource **head, u32 size *head = node->next; } else { prevnode = *head; - while (prevnode->next != node) + while (prevnode && prevnode->next != node) prevnode = prevnode->next; - prevnode->next = node->next; + if (prevnode) + prevnode->next = node->next; } node->next = NULL; break; @@ -788,10 +789,11 @@ static struct pci_resource *get_resource(struct pci_resource **head, u32 size) *head = node->next; } else { prevnode = *head; - while (prevnode->next != node) + while (prevnode && prevnode->next != node) prevnode = prevnode->next; - prevnode->next = node->next; + if (prevnode) + prevnode->next = node->next; } node->next = NULL; break;
There is otherwise a risk of a null pointer dereference. Signed-off-by: Young Xiao <92siuyang@gmail.com> --- drivers/pci/hotplug/cpqphp_ctrl.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)