| Message ID | 20231009222930.10020-2-asmaa@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | UBUNTU: SAUCE: gpio-mlxbf3: support valid mask | expand |
On 10/9/23 4:29 PM, Asmaa Mnebhi wrote: > BugLink: https://bugs.launchpad.net/bugs/2038868 > > After syncing up the gpio-mlxbf3.c driver with the upstreamed version, > we dropped the use of the valid_mask variable because kernels greater > or equal to 6.2.0 don't need it. > This is no longer needed in kernel versions >= 6.2.0 because valid_mask > is populated by core gpio code. > 5.15 kernel, doesn't support that feature so we still need to explicitly > define valid_mask as we did before. > This doesnt impact the functionality of the GPIO driver but it is a > security breach as it doesnt restrict the access to gpios defined in the > acpi table by valid_mask. > > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> > --- > drivers/gpio/gpio-mlxbf3.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c > index e63b0e7e90bd..e1375756c487 100644 > --- a/drivers/gpio/gpio-mlxbf3.c > +++ b/drivers/gpio/gpio-mlxbf3.c > @@ -49,6 +49,9 @@ struct mlxbf3_gpio_context { > > /* YU GPIO cause block address */ > void __iomem *gpio_cause_io; > + > + /* Mask of valid gpios that can be accessed by software */ > + unsigned int valid_mask; > }; > > static void mlxbf3_gpio_irq_enable(struct irq_data *irqd) > @@ -150,6 +153,17 @@ static void mlxbf3_gpio_irq_ack(struct irq_data *data) > { > } > > +static int mlxbf3_gpio_init_valid_mask(struct gpio_chip *gc, > + unsigned long *valid_mask, > + unsigned int ngpios) > +{ > + struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc); > + > + *valid_mask = gs->valid_mask; > + > + return 0; > +} > + > static struct irq_chip gpio_mlxbf3_irqchip = { > .name = "MLNXBF33", > .irq_ack = mlxbf3_gpio_irq_ack, > @@ -205,6 +219,10 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) > gs->gpio_clr_io = devm_platform_ioremap_resource(pdev, 3); > if (IS_ERR(gs->gpio_clr_io)) > return PTR_ERR(gs->gpio_clr_io); > + > + gs->valid_mask = 0x0; Isn't this ^ superfluous ? It appears that gs->valid_mask is overwritten in the call to device_property_read_u32(). > + device_property_read_u32(dev, "valid_mask", &gs->valid_mask); > + > gc = &gs->gc; > > ret = bgpio_init(gc, dev, 4, > @@ -218,6 +236,7 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) > gc->free = gpiochip_generic_free; > gc->owner = THIS_MODULE; > gc->add_pin_ranges = mlxbf3_gpio_add_pin_ranges; > + gc->init_valid_mask = mlxbf3_gpio_init_valid_mask; > > irq = platform_get_irq(pdev, 0); > if (irq >= 0) {
> > + gs->valid_mask = 0x0; > > Isn't this ^ superfluous ? It appears that gs->valid_mask is overwritten in the > call to device_property_read_u32(). > > > + device_property_read_u32(dev, "valid_mask", > > +&gs->valid_mask); > > + Thanks for catching that. Sent v2.
diff --git a/drivers/gpio/gpio-mlxbf3.c b/drivers/gpio/gpio-mlxbf3.c index e63b0e7e90bd..e1375756c487 100644 --- a/drivers/gpio/gpio-mlxbf3.c +++ b/drivers/gpio/gpio-mlxbf3.c @@ -49,6 +49,9 @@ struct mlxbf3_gpio_context { /* YU GPIO cause block address */ void __iomem *gpio_cause_io; + + /* Mask of valid gpios that can be accessed by software */ + unsigned int valid_mask; }; static void mlxbf3_gpio_irq_enable(struct irq_data *irqd) @@ -150,6 +153,17 @@ static void mlxbf3_gpio_irq_ack(struct irq_data *data) { } +static int mlxbf3_gpio_init_valid_mask(struct gpio_chip *gc, + unsigned long *valid_mask, + unsigned int ngpios) +{ + struct mlxbf3_gpio_context *gs = gpiochip_get_data(gc); + + *valid_mask = gs->valid_mask; + + return 0; +} + static struct irq_chip gpio_mlxbf3_irqchip = { .name = "MLNXBF33", .irq_ack = mlxbf3_gpio_irq_ack, @@ -205,6 +219,10 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) gs->gpio_clr_io = devm_platform_ioremap_resource(pdev, 3); if (IS_ERR(gs->gpio_clr_io)) return PTR_ERR(gs->gpio_clr_io); + + gs->valid_mask = 0x0; + device_property_read_u32(dev, "valid_mask", &gs->valid_mask); + gc = &gs->gc; ret = bgpio_init(gc, dev, 4, @@ -218,6 +236,7 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev) gc->free = gpiochip_generic_free; gc->owner = THIS_MODULE; gc->add_pin_ranges = mlxbf3_gpio_add_pin_ranges; + gc->init_valid_mask = mlxbf3_gpio_init_valid_mask; irq = platform_get_irq(pdev, 0); if (irq >= 0) {
BugLink: https://bugs.launchpad.net/bugs/2038868 After syncing up the gpio-mlxbf3.c driver with the upstreamed version, we dropped the use of the valid_mask variable because kernels greater or equal to 6.2.0 don't need it. This is no longer needed in kernel versions >= 6.2.0 because valid_mask is populated by core gpio code. 5.15 kernel, doesn't support that feature so we still need to explicitly define valid_mask as we did before. This doesnt impact the functionality of the GPIO driver but it is a security breach as it doesnt restrict the access to gpios defined in the acpi table by valid_mask. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> --- drivers/gpio/gpio-mlxbf3.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)