From ef2bf4997f7da6efa8540d9cf726c44bf2b863af Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Fri, 27 May 2016 09:45:49 -0700 Subject: pwm: Improve args checking in pwm_apply_state() It seems like in the process of refactoring pwm_config() to utilize the newly-introduced pwm_apply_state() API, some args/bounds checking was dropped. In particular, I noted that we are now allowing invalid period selections, e.g.: # echo 1 > /sys/class/pwm/pwmchip0/export # cat /sys/class/pwm/pwmchip0/pwm1/period 100 # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle [... driver may or may not reject the value, or trigger some logic bug ...] It's better to see: # echo 1 > /sys/class/pwm/pwmchip0/export # cat /sys/class/pwm/pwmchip0/pwm1/period 100 # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle -bash: echo: write error: Invalid argument This patch reintroduces some bounds checks in both pwm_config() (for its signed parameters; we don't want to convert negative values into large unsigned values) and in pwm_apply_state() (which fix the above described behavior, as well as other potential API misuses). Fixes: 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates") Signed-off-by: Brian Norris Acked-by: Boris Brezillon Signed-off-by: Thierry Reding --- include/linux/pwm.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include/linux/pwm.h') diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 17018f3c066e..908b67c847cd 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -235,6 +235,9 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns, if (!pwm) return -EINVAL; + if (duty_ns < 0 || period_ns < 0) + return -EINVAL; + pwm_get_state(pwm, &state); if (state.duty_cycle == duty_ns && state.period == period_ns) return 0; -- cgit v1.2.3 From 33cdcee04be3b4482be97393167e7561b2584e1e Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Wed, 22 Jun 2016 09:25:14 +0200 Subject: pwm: Fix pwm_apply_args() Commit 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates"), implemented pwm_disable() as a wrapper around pwm_apply_state(), and then, commit ef2bf4997f7d ("pwm: Improve args checking in pwm_apply_state()") added missing checks on the ->period value in pwm_apply_state() to ensure we were not passing inappropriate values to the ->config() or ->apply() methods. The conjunction of these 2 commits led to a case where pwm_disable() was no longer succeeding, thus preventing the polarity setting done in pwm_apply_args(). Set a valid period in pwm_apply_args() to ensure polarity setting won't be rejected. Signed-off-by: Boris Brezillon Reported-by: Geert Uytterhoeven Suggested-by: Brian Norris Fixes: 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates") Tested-by: Geert Uytterhoeven Reviewed-by: Brian Norris Signed-off-by: Thierry Reding --- include/linux/pwm.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'include/linux/pwm.h') diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 908b67c847cd..c038ae36b10e 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -464,6 +464,8 @@ static inline bool pwm_can_sleep(struct pwm_device *pwm) static inline void pwm_apply_args(struct pwm_device *pwm) { + struct pwm_state state = { }; + /* * PWM users calling pwm_apply_args() expect to have a fresh config * where the polarity and period are set according to pwm_args info. @@ -476,18 +478,20 @@ static inline void pwm_apply_args(struct pwm_device *pwm) * at startup (even if they are actually enabled), thus authorizing * polarity setting. * - * Instead of setting ->enabled to false, we call pwm_disable() - * before pwm_set_polarity() to ensure that everything is configured - * as expected, and the PWM is really disabled when the user request - * it. + * To fulfill this requirement, we apply a new state which disables + * the PWM device and set the reference period and polarity config. * * Note that PWM users requiring a smooth handover between the * bootloader and the kernel (like critical regulators controlled by * PWM devices) will have to switch to the atomic API and avoid calling * pwm_apply_args(). */ - pwm_disable(pwm); - pwm_set_polarity(pwm, pwm->args.polarity); + + state.enabled = false; + state.polarity = pwm->args.polarity; + state.period = pwm->args.period; + + pwm_apply_state(pwm, &state); } struct pwm_lookup { -- cgit v1.2.3