linux-kernel - Re: [PATCH] pwm: removes period check from pwm_apply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220805155509.edqwxcvyoqfic4pn@pengutronix.de>
Date:   Fri, 5 Aug 2022 17:55:09 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Tamseel Shams <m.shams@...sung.com>
Cc:     thierry.reding@...il.com, lee.jones@...aro.org,
        linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
        alim.akhtar@...sung.com
Subject: Re: [PATCH] pwm: removes period check from pwm_apply_state()

Hello,

On Fri, Aug 05, 2022 at 03:41:25PM +0530, Tamseel Shams wrote:
> There may be situation when PWM is exported using sysfs,
> but at that point PWM period is not set. At this situation
> if we issue a system suspend, it calls pwm_class_suspend
> which in turn calls pwm_apply_state, where PWM period value is
> checked which returns an invalid argument error casuing Kernel
> to panic. So, check for PWM period value is removed so as to
> fix the kernel panic observed during suspend.

This looks and sounds wrong. One thing I would accept is:

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 0e042410f6b9..075bbcdad6c1 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -557,8 +557,8 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
 	 */
 	might_sleep();
 
-	if (!pwm || !state || !state->period ||
-	    state->duty_cycle > state->period)
+	if (!pwm || !state || state->enabled && (!state->period ||
+	    state->duty_cycle > state->period))
 		return -EINVAL;
 
 	chip = pwm->chip;

That is, don't refuse calling pwm_apply_state() for state->period = 0
and even state->duty_cycle > state->period if the PWM is not enabled.

But anyhow, even without that the kernel should not panic. So I ask you
to research and provide some more info about the problem. (Which
hardware does it affect? Where does it panic? ...)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)