[BUG] [Resend] Possible sleep-in-atomic-context bugs involving regmap_lock_mutex()

[BUG] [Resend] Possible sleep-in-atomic-context bugs involving regmap_lock_mutex()

[Jia-Ju Bai]

Hello,

My static tool DSAC reports many sleep-in-atomic-context bugs involving 
regmap_lock_mutex(), so I wonder whether this function is possible to be 
executed in atomic context.

Here are some example bugs and their call paths in Linux-4.16 (from 
bottom to top, and [FUNC_PTR] means that there is a function pointer call):

[FUNC] mutex_lock_nested
drivers/base/regmap/regmap.c, 468:
     mutex_lock_nested in regmap_lock_mutex
drivers/base/regmap/regmap.c, 2503:
     [FUNC_PTR]regmap_lock_mutex in regmap_read
drivers/clk/clk-aspeed.c, 215:
     regmap_read in aspeed_clk_is_enabled
drivers/clk/clk-aspeed.c, 230:
     aspeed_clk_is_enabled in aspeed_clk_enable
drivers/clk/clk-aspeed.c, 228:
     _raw_spin_lock_irqsave in aspeed_clk_enable

[FUNC] mutex_lock_nested
drivers/base/regmap/regmap.c, 468:
     mutex_lock_nested in regmap_lock_mutex
drivers/base/regmap/regmap.c, 2821:
     [FUNC_PTR]regmap_lock_mutex in regmap_update_bits_base
drivers/clk/clk-aspeed.c, 270:
     regmap_update_bits_base in aspeed_clk_disable
drivers/clk/clk-aspeed.c, 267:
     _raw_spin_lock_irqsave in aspeed_clk_disable

[FUNC] mutex_lock_nested
drivers/base/regmap/regmap.c, 468:
     mutex_lock_nested in regmap_lock_mutex
drivers/base/regmap/regmap.c, 2503:
     [FUNC_PTR]regmap_lock_mutex in regmap_read
drivers/char/ipmi/bt-bmc.c, 385:
     regmap_read in bt_bmc_irq

[FUNC] mutex_lock_nested
drivers/base/regmap/regmap.c, 468:
     mutex_lock_nested in regmap_lock_mutex
drivers/base/regmap/regmap.c, 2821:
     [FUNC_PTR]regmap_lock_mutex in regmap_update_bits_base
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c, 213:
     regmap_update_bits_base in hdmi_modb
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c, 429:
     hdmi_modb in hdmi_set_cts_n
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c, 527:
     hdmi_set_cts_n in hdmi_set_clk_regenerator
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c, 524:
     spin_lock_irq in hdmi_set_clk_regenerator

[FUNC] mutex_lock_nested
drivers/base/regmap/regmap.c, 468:
     mutex_lock_nested in regmap_lock_mutex
drivers/base/regmap/regmap.c, 2503:
     [FUNC_PTR]regmap_lock_mutex in regmap_read
drivers/media/platform/atmel/atmel-isc.c, 1603:
     regmap_read in isc_interrupt (interrupt handler)

I find the code about the assignment of regmap_lock_mutex():
         if ((bus && bus->fast_io) ||
             config->fast_io) {
             spin_lock_init(&map->spinlock);
             map->lock = regmap_lock_spinlock;
             map->unlock = regmap_unlock_spinlock;
             lockdep_set_class_and_name(&map->spinlock,
                            lock_key, lock_name);
         } else {
             mutex_init(&map->mutex);
             map->lock = regmap_lock_mutex;
             map->unlock = regmap_unlock_mutex;
             lockdep_set_class_and_name(&map->mutex,
                            lock_key, lock_name);
         }

But after reading the code, I cannot find the relationship between the 
if condition and atomic context.
I think assigning regmap_lock_mutex() to map->lock may be not proper.

I also find some similar previously reported problems about 
regmap_lock_mutex():
https://lists.launchpad.net/kernel-packages/msg187700.html
https://community.nxp.com/thread/432071

I am not sure whether my analysis and results are right.
Could someone please give me explanation?

Thanks in advance :)


Best wishes,
Jia-Ju Bai

Re: [BUG] [Resend] Possible sleep-in-atomic-context bugs involving regmap_lock_mutex()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

On Thu, Aug 30, 2018 at 10:34:20AM +0800, Jia-Ju Bai wrote:

> My static tool DSAC reports many sleep-in-atomic-context bugs involving
> regmap_lock_mutex(), so I wonder whether this function is possible to be
> executed in atomic context.

Have you actually analyzed the code paths that are really taken here?
Static tools really don't cope very well with the pluggable locking in
regmap.  If there are problems I'd suggest reporting them to the authors
of the drivers using them, it's not really anything to do with regmap if
they've messed up their locking.

> Here are some example bugs and their call paths in Linux-4.16 (from bottom
> to top, and [FUNC_PTR] means that there is a function pointer call):

That's quite old now...  not that this has been rapidly developed
recently but still.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG] [Resend] Possible sleep-in-atomic-context bugs involving regmap_lock_mutex()

[Jia-Ju Bai]

Thanks for the reply :)

On 2018/9/11 1:41, Mark Brown wrote:
> On Thu, Aug 30, 2018 at 10:34:20AM +0800, Jia-Ju Bai wrote:
>
>> My static tool DSAC reports many sleep-in-atomic-context bugs involving
>> regmap_lock_mutex(), so I wonder whether this function is possible to be
>> executed in atomic context.
> Have you actually analyzed the code paths that are really taken here?
> Static tools really don't cope very well with the pluggable locking in
> regmap.  If there are problems I'd suggest reporting them to the authors
> of the drivers using them, it's not really anything to do with regmap if
> they've messed up their locking.

Okay, I will send the reports to the developers for these drivers.


Best wishes,
Jia-Ju Bai