Re: [PATCH] random: Fix the issue of '_might_sleep' function running in an atomic contex

From: Guoyong Wang
Date: Wed Mar 20 2024 - 05:03:30 EST


On Web, 20 Mar 2024 02:09:21 +0100, Jason A. Donenfeld wrote:
>> Hi Jason,
>>
>> Thanks for your suggestions.
>>
>> I am inclined to accept your second suggestion. My reluctance to accept
>> the first is due to the concern that "&& !in_atomic()" could potentially
>> alter the original meaning of the 'execute_in_process_context' interface.
>> Regarding the third suggestion, modifying the logic associated with 'input'
>> is not recommended.
>
> Doesn't something like the below seem simplest? Just move the call out
> of the spinlock and we're done.
>
> diff --git a/drivers/input/input-core-private.h b/drivers/input/input-core-private.h
> index 116834cf8868..717f239e28d0 100644
> --- a/drivers/input/input-core-private.h
> +++ b/drivers/input/input-core-private.h
> @@ -10,7 +10,7 @@
> struct input_dev;
>
> void input_mt_release_slots(struct input_dev *dev);
> -void input_handle_event(struct input_dev *dev,
> +bool input_handle_event(struct input_dev *dev,
> unsigned int type, unsigned int code, int value);
>
> #endif /* _INPUT_CORE_PRIVATE_H */
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index f71ea4fb173f..2faf46218c66 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -391,20 +391,22 @@ static void input_event_dispose(struct input_dev *dev, int disposition,
> }
> }
>
> -void input_handle_event(struct input_dev *dev,
> +bool input_handle_event(struct input_dev *dev,
> unsigned int type, unsigned int code, int value)
> {
> int disposition;
> +bool should_contribute_to_rng = false;
>
> lockdep_assert_held(&dev->event_lock);
>
> disposition = input_get_disposition(dev, type, code, &value);
> if (disposition != INPUT_IGNORE_EVENT) {
> if (type != EV_SYN)
> -add_input_randomness(type, code, value);
> +should_contribute_to_rng = true;
>
> input_event_dispose(dev, disposition, type, code, value);
> }
> +return should_contribute_to_rng;
> }
>
> /**
> @@ -428,12 +430,15 @@ void input_event(struct input_dev *dev,
> unsigned int type, unsigned int code, int value)
> {
> unsigned long flags;
> +bool should_contribute_to_rng;
>
> if (is_event_supported(type, dev->evbit, EV_MAX)) {
>
> spin_lock_irqsave(&dev->event_lock, flags);
> -input_handle_event(dev, type, code, value);
> +should_contribute_to_rng = input_handle_event(dev, type, code, value);
> spin_unlock_irqrestore(&dev->event_lock, flags);
> +if (should_contribute_to_rng)
> +add_input_randomness(type, code, value);
> }
> }
> EXPORT_SYMBOL(input_event);

Hi Jason,

Your proposal is not suitable for scenarios where input_event is called within an atomic context.

For example:
spin_lock(&dev->spinlock);
input_event(dev, EV_ABS, ABS_X, x);
spin_unlock(&dev->spinlock);