| Message ID | 20230315061121.1741454-1-windhl@126.com |
|---|---|
| State | New |
| Headers | show |
| Series | tty: vcc: add check for mdesc_grab() | expand |
On Wed, Mar 15, 2023 at 02:11:21PM +0800, Liang He wrote: > In vcc_probe(), we should check the return value of > mdesc_grab() as it may return NULL. While the > vio_vdev_node() has the NULL-check, but if there > is still a call to mdesc_release() which may cause > a NPD bug. Have you actually triggered this issue? If so, how? > Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal") > Signed-off-by: Liang He <windhl@126.com> > --- > drivers/tty/vcc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c > index 34ba6e54789a..e3ba63d0a91f 100644 > --- a/drivers/tty/vcc.c > +++ b/drivers/tty/vcc.c > @@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id) > > hp = mdesc_grab(); > > + if (!hp) > + return -ENODEV; This change is obviously not correct and has not been tested, sorry. Please do not make changes like this without properly validating them. greg k-h
At 2023-03-15 14:32:54, "Greg KH" <gregkh@linuxfoundation.org> wrote: >On Wed, Mar 15, 2023 at 02:11:21PM +0800, Liang He wrote: >> In vcc_probe(), we should check the return value of >> mdesc_grab() as it may return NULL. While the >> vio_vdev_node() has the NULL-check, but if there >> is still a call to mdesc_release() which may cause >> a NPD bug. > >Have you actually triggered this issue? If so, how? > Hi, Greg, Thanks very much for your reply. In fact, I have not actually triggered this issue, but I indeed meet lots of checks in other *_init functions, e.g., mdesc_adi_init()/ldc_init(). However, if we can make sure the return value can never be NULL when our kernel execute into these *_probe functions, my patchs are indeed useless. Thanks and sorry for any potential trouble. Liang >> Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal") >> Signed-off-by: Liang He <windhl@126.com> >> --- >> drivers/tty/vcc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c >> index 34ba6e54789a..e3ba63d0a91f 100644 >> --- a/drivers/tty/vcc.c >> +++ b/drivers/tty/vcc.c >> @@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id) >> >> hp = mdesc_grab(); >> >> + if (!hp) >> + return -ENODEV; > >This change is obviously not correct and has not been tested, sorry. >Please do not make changes like this without properly validating them. > >greg k-h
diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c index 34ba6e54789a..e3ba63d0a91f 100644 --- a/drivers/tty/vcc.c +++ b/drivers/tty/vcc.c @@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id) hp = mdesc_grab(); + if (!hp) + return -ENODEV; + node = vio_vdev_node(hp, vdev); if (node == MDESC_NODE_NULL) { rv = -ENXIO;
In vcc_probe(), we should check the return value of mdesc_grab() as it may return NULL. While the vio_vdev_node() has the NULL-check, but if there is still a call to mdesc_release() which may cause a NPD bug. Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal") Signed-off-by: Liang He <windhl@126.com> --- drivers/tty/vcc.c | 3 +++ 1 file changed, 3 insertions(+)