| Message ID | 20220418093144.40859-1-hbh25y@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | rpmsg: virtio: fix possible double free in rpmsg_probe() | expand |
On 4/18/22 11:31, Hangyu Hua wrote: > vch will be free in virtio_rpmsg_release_device() when > rpmsg_ns_register_device() fails. There is no need to call kfree() again. > > Fix this by changing error path from free_vch to free_ctrldev. > > Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device") > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> Tested-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> Thanks, Arnaud > --- > drivers/rpmsg/virtio_rpmsg_bus.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > index 3ede25b1f2e4..603233f0686e 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -973,7 +973,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > > err = rpmsg_ns_register_device(rpdev_ns); > if (err) > - goto free_vch; > + goto free_ctrldev; > } > > /* > @@ -997,8 +997,6 @@ static int rpmsg_probe(struct virtio_device *vdev) > > return 0; > > -free_vch: > - kfree(vch); > free_ctrldev: > rpmsg_virtio_del_ctrl_dev(rpdev_ctrl); > free_coherent:
On Mon, Apr 18, 2022 at 05:31:44PM +0800, Hangyu Hua wrote: > vch will be free in virtio_rpmsg_release_device() when > rpmsg_ns_register_device() fails. There is no need to call kfree() again. > > Fix this by changing error path from free_vch to free_ctrldev. > > Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device") > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > --- > drivers/rpmsg/virtio_rpmsg_bus.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c > index 3ede25b1f2e4..603233f0686e 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -973,7 +973,7 @@ static int rpmsg_probe(struct virtio_device *vdev) > > err = rpmsg_ns_register_device(rpdev_ns); > if (err) > - goto free_vch; Please add a comment that highlights where 'vch' will be free'd to avoid receiving patches that will introduce another kfree(). Same for your other patch. In the next revision please use a cover letter and add Arnaud's patches to it. Thanks, Mathieu > + goto free_ctrldev; > } > > /* > @@ -997,8 +997,6 @@ static int rpmsg_probe(struct virtio_device *vdev) > > return 0; > > -free_vch: > - kfree(vch); > free_ctrldev: > rpmsg_virtio_del_ctrl_dev(rpdev_ctrl); > free_coherent: > -- > 2.25.1 >
On 2022/4/26 00:55, Mathieu Poirier wrote: > On Mon, Apr 18, 2022 at 05:31:44PM +0800, Hangyu Hua wrote: >> vch will be free in virtio_rpmsg_release_device() when >> rpmsg_ns_register_device() fails. There is no need to call kfree() again. >> >> Fix this by changing error path from free_vch to free_ctrldev. >> >> Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device") >> Signed-off-by: Hangyu Hua <hbh25y@gmail.com> >> --- >> drivers/rpmsg/virtio_rpmsg_bus.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c >> index 3ede25b1f2e4..603233f0686e 100644 >> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >> @@ -973,7 +973,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >> >> err = rpmsg_ns_register_device(rpdev_ns); >> if (err) >> - goto free_vch; > > Please add a comment that highlights where 'vch' will be free'd to avoid > receiving patches that will introduce another kfree(). Same for your other > patch. > > In the next revision please use a cover letter and add Arnaud's patches to it. > > Thanks, > Mathieu > Thanks! I will send a v2 later. >> + goto free_ctrldev; >> } >> >> /* >> @@ -997,8 +997,6 @@ static int rpmsg_probe(struct virtio_device *vdev) >> >> return 0; >> >> -free_vch: >> - kfree(vch); >> free_ctrldev: >> rpmsg_virtio_del_ctrl_dev(rpdev_ctrl); >> free_coherent: >> -- >> 2.25.1 >>
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c index 3ede25b1f2e4..603233f0686e 100644 --- a/drivers/rpmsg/virtio_rpmsg_bus.c +++ b/drivers/rpmsg/virtio_rpmsg_bus.c @@ -973,7 +973,7 @@ static int rpmsg_probe(struct virtio_device *vdev) err = rpmsg_ns_register_device(rpdev_ns); if (err) - goto free_vch; + goto free_ctrldev; } /* @@ -997,8 +997,6 @@ static int rpmsg_probe(struct virtio_device *vdev) return 0; -free_vch: - kfree(vch); free_ctrldev: rpmsg_virtio_del_ctrl_dev(rpdev_ctrl); free_coherent:
vch will be free in virtio_rpmsg_release_device() when rpmsg_ns_register_device() fails. There is no need to call kfree() again. Fix this by changing error path from free_vch to free_ctrldev. Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device") Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- drivers/rpmsg/virtio_rpmsg_bus.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)