| Message ID | 20240821062132.1407444-1-make24@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] pinctrl: single: fix potential NULL dereference in pcs_get_function() | expand |
On Wed, 21 Aug 2024 14:21:32 +0800 Ma Ke <make24@iscas.ac.cn> wrote: > pinmux_generic_get_function() can return NULL and the pointer 'function' > was dereferenced without checking against NULL. Add checking of pointer > 'function' in pcs_get_function(). > > Found by code review. > > ... > > --- a/drivers/pinctrl/pinctrl-single.c > +++ b/drivers/pinctrl/pinctrl-single.c > @@ -345,6 +345,8 @@ static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin, > return -ENOTSUPP; > fselector = setting->func; > function = pinmux_generic_get_function(pctldev, fselector); > + if (!function) > + return -EINVAL; > *func = function->data; > if (!(*func)) { > dev_err(pcs->dev, "%s could not find function%i\n", Maybe. Or maybe pinmux_generic_get_function() must always return a valid pointer, in which case BUG_ON(!function); is an appropriate thing. But a null-pointer deref gives us the same info, so no change is needed. btw, pinmux_generic_get_function() is funny: if (!function) return NULL; return function;
Andrew Morton<akpm@linux-foundation.org> wrote: > On Wed, 21 Aug 2024 14:21:32 +0800 Ma Ke <make24@iscas.ac.cn> wrote: > > > pinmux_generic_get_function() can return NULL and the pointer 'function' > > was dereferenced without checking against NULL. Add checking of pointer > > 'function' in pcs_get_function(). > > > > Found by code review. > > > > ... > > > > --- a/drivers/pinctrl/pinctrl-single.c > > +++ b/drivers/pinctrl/pinctrl-single.c > > @@ -345,6 +345,8 @@ static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin, > > return -ENOTSUPP; > > fselector = setting->func; > > function = pinmux_generic_get_function(pctldev, fselector); > > + if (!function) > > + return -EINVAL; > > *func = function->data; > > if (!(*func)) { > > dev_err(pcs->dev, "%s could not find function%i\n", > > Maybe. Or maybe pinmux_generic_get_function() must always return a > valid pointer, in which case > > BUG_ON(!function); > > is an appropriate thing. But a null-pointer deref gives us the same > info, so no change is needed. > > btw, pinmux_generic_get_function() is funny: > > if (!function) > return NULL; > > return function; Thank you for your response to the vulnerability I submitted. Yes, we believe there is a similar issue. As described in [1], pinmux_generic_get_function() could return as NULL and lead to a d ereferencing problem, and a similar issue exists in this code. It is better to add checking of pointer 'function' in pcs_get_function(). The discovery of this problem was confirmed through manual review of the code and compilation testing. [1] https://lore.kernel.org/linux-arm-kernel/CACRpkdYwBNjGzODYqvz+oScsO3u=R0dXMkP4UfqmosDugPFWRA@mail.gmail.com/T/ -- Regards, Ma Ke
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 4c6bfabb6bd7..4da3c3f422b6 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -345,6 +345,8 @@ static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin, return -ENOTSUPP; fselector = setting->func; function = pinmux_generic_get_function(pctldev, fselector); + if (!function) + return -EINVAL; *func = function->data; if (!(*func)) { dev_err(pcs->dev, "%s could not find function%i\n",
pinmux_generic_get_function() can return NULL and the pointer 'function' was dereferenced without checking against NULL. Add checking of pointer 'function' in pcs_get_function(). Found by code review. Cc: stable@vger.kernel.org Fixes: 571aec4df5b7 ("pinctrl: single: Use generic pinmux helpers for managing functions") Signed-off-by: Ma Ke <make24@iscas.ac.cn> --- drivers/pinctrl/pinctrl-single.c | 2 ++ 1 file changed, 2 insertions(+)