| Message ID | 20201005213149.12332-1-sudipm.mukherjee@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: host: ehci-sched: avoid possible NULL dereference | expand |
On Monday, October 5, 2020 5:31 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > find_tt() can return NULL or the error value in ERR_PTR() and > dereferencing the return value without checking for the error can > lead to a possible dereference of NULL pointer or ERR_PTR(). Looks fine to me. There is in fact no checks of the return value before a dereference here, and this solves that. Reviewed-by: Harley A.W. Lorenzo <hl1998@protonmail.com
On Mon, Oct 05, 2020 at 11:19:02PM +0000, Harley A.W. Lorenzo wrote: > On Monday, October 5, 2020 5:31 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > > > find_tt() can return NULL or the error value in ERR_PTR() and > > dereferencing the return value without checking for the error can > > lead to a possible dereference of NULL pointer or ERR_PTR(). > > Looks fine to me. There is in fact no checks of the return value > before a dereference here, and this solves that. > > Reviewed-by: Harley A.W. Lorenzo <hl1998@protonmail.com No, this patch is wrong. In fact, these calls to find_tt() cannot return NULL or an ERR_PTR value. Alan Stern
On Mon, Oct 05, 2020 at 11:19:02PM +0000, Harley A.W. Lorenzo wrote: > On Monday, October 5, 2020 5:31 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > > > find_tt() can return NULL or the error value in ERR_PTR() and > > dereferencing the return value without checking for the error can > > lead to a possible dereference of NULL pointer or ERR_PTR(). > > Looks fine to me. There is in fact no checks of the return value > before a dereference here, and this solves that. > > Reviewed-by: Harley A.W. Lorenzo <hl1998@protonmail.com Nit, in the future, you need the trailing '>' there. thanks, greg k-h
On Mon, Oct 05, 2020 at 09:25:44PM -0400, stern@rowland.harvard.edu wrote: > On Mon, Oct 05, 2020 at 11:19:02PM +0000, Harley A.W. Lorenzo wrote: > > On Monday, October 5, 2020 5:31 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > > > > > find_tt() can return NULL or the error value in ERR_PTR() and > > > dereferencing the return value without checking for the error can > > > lead to a possible dereference of NULL pointer or ERR_PTR(). > > > > Looks fine to me. There is in fact no checks of the return value > > before a dereference here, and this solves that. > > > > Reviewed-by: Harley A.W. Lorenzo <hl1998@protonmail.com > > No, this patch is wrong. In fact, these calls to find_tt() cannot > return NULL or an ERR_PTR value. Sudip, if you would prefer to submit a patch that adds comments to those call sites explaining that find_tt() will not return NULL or an error, that would be okay. Alan Stern
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c index 6dfb242f9a4b..f3fd7e9fe6b2 100644 --- a/drivers/usb/host/ehci-sched.c +++ b/drivers/usb/host/ehci-sched.c @@ -245,6 +245,8 @@ static void reserve_release_intr_bandwidth(struct ehci_hcd *ehci, /* FS/LS bus bandwidth */ if (tt_usecs) { tt = find_tt(qh->ps.udev); + if (IS_ERR_OR_NULL(tt)) + return; if (sign > 0) list_add_tail(&qh->ps.ps_list, &tt->ps_list); else @@ -1338,6 +1340,8 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci, } tt = find_tt(stream->ps.udev); + if (IS_ERR_OR_NULL(tt)) + return; if (sign > 0) list_add_tail(&stream->ps.ps_list, &tt->ps_list); else
find_tt() can return NULL or the error value in ERR_PTR() and dereferencing the return value without checking for the error can lead to a possible dereference of NULL pointer or ERR_PTR(). Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com> --- drivers/usb/host/ehci-sched.c | 4 ++++ 1 file changed, 4 insertions(+)