* [linux-next:master 1131/2532] drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
@ 2024-03-31 23:09 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-03-31 23:09 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
CC: Linux Memory Management List <linux-mm@kvack.org>
TO: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
commit: 0835e5c8704297e91b54071c3bbcd0c05c63bd3f [1131/2532] pwm: Add support for pwmchip devices for faster and easier userspace access
:::::: branch date: 4 days ago
:::::: commit date: 7 days ago
config: m68k-randconfig-r071-20240326 (https://download.01.org/0day-ci/archive/20240401/202404010703.o1yYkXrA-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202404010703.o1yYkXrA-lkp@intel.com/
New smatch warnings:
drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
drivers/pwm/core.c:1284 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
drivers/pwm/core.c:1368 pwm_cdev_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?
Old smatch warnings:
drivers/pwm/core.c:1357 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
drivers/pwm/core.c:1871 pwm_put() warn: variable dereferenced before check 'pwm' (see line 1869)
vim +1282 drivers/pwm/core.c
0835e5c8704297 Uwe Kleine-König 2024-03-17 1225
0835e5c8704297 Uwe Kleine-König 2024-03-17 1226 static long pwm_cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1227 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1228 int ret = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1229 struct pwm_cdev_data *cdata = file->private_data;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1230 struct pwm_chip *chip = cdata->chip;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1231
0835e5c8704297 Uwe Kleine-König 2024-03-17 1232 mutex_lock(&pwm_lock);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1233
0835e5c8704297 Uwe Kleine-König 2024-03-17 1234 if (!chip->operational) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1235 ret = -ENODEV;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1236 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1237 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1238
0835e5c8704297 Uwe Kleine-König 2024-03-17 1239 switch (cmd) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1240 case PWM_IOCTL_GET_NUM_PWMS:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1241 ret = chip->npwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1242 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1243
0835e5c8704297 Uwe Kleine-König 2024-03-17 1244 case PWM_IOCTL_REQUEST:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1245 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1246 unsigned int hwpwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1247
0835e5c8704297 Uwe Kleine-König 2024-03-17 1248 ret = get_user(hwpwm, (unsigned int __user *)arg);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1249 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1250 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1251
0835e5c8704297 Uwe Kleine-König 2024-03-17 1252 ret = pwm_cdev_request(cdata, hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1253 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1254 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1255
0835e5c8704297 Uwe Kleine-König 2024-03-17 1256 case PWM_IOCTL_FREE:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1257 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1258 unsigned int hwpwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1259
0835e5c8704297 Uwe Kleine-König 2024-03-17 1260 ret = get_user(hwpwm, (unsigned int __user *)arg);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1261 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1262 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1263
0835e5c8704297 Uwe Kleine-König 2024-03-17 1264 ret = pwm_cdev_free(cdata, hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1265 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1266 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1267
0835e5c8704297 Uwe Kleine-König 2024-03-17 1268 case PWM_IOCTL_GET:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1269 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1270 struct pwmchip_state cstate;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1271 struct pwm_state state;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1272 struct pwm_device *pwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1273
0835e5c8704297 Uwe Kleine-König 2024-03-17 1274 ret = copy_from_user(&cstate, (struct pwmchip_state __user *)arg, sizeof(cstate));
0835e5c8704297 Uwe Kleine-König 2024-03-17 1275 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1276 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1277
0835e5c8704297 Uwe Kleine-König 2024-03-17 1278 ret = pwm_cdev_request(cdata, cstate.hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1279 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1280 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1281
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1282 pwm = cdata->pwm[cstate.hwpwm];
0835e5c8704297 Uwe Kleine-König 2024-03-17 1283
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1284 ret = pwm_get_state_hw(pwm, &state);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1285 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1286 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1287
0835e5c8704297 Uwe Kleine-König 2024-03-17 1288 if (state.enabled) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1289 cstate.period = state.period;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1290 if (state.polarity == PWM_POLARITY_NORMAL) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1291 cstate.duty_offset = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1292 cstate.duty_cycle = state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1293 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1294 cstate.duty_offset = state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1295 cstate.duty_cycle = state.period - state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1296 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1297 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1298 cstate.period = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1299 cstate.duty_cycle = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1300 cstate.duty_offset = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1301 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1302 ret = copy_to_user((struct pwmchip_state __user *)arg, &cstate, sizeof(cstate));
0835e5c8704297 Uwe Kleine-König 2024-03-17 1303 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1304 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1305
0835e5c8704297 Uwe Kleine-König 2024-03-17 1306 case PWM_IOCTL_APPLY:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1307 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1308 struct pwmchip_state cstate;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1309 struct pwm_state state = { };
0835e5c8704297 Uwe Kleine-König 2024-03-17 1310 struct pwm_device *pwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1311
0835e5c8704297 Uwe Kleine-König 2024-03-17 1312 ret = copy_from_user(&cstate, (struct pwmchip_state __user *)arg, sizeof(cstate));
0835e5c8704297 Uwe Kleine-König 2024-03-17 1313 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1314 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1315
0835e5c8704297 Uwe Kleine-König 2024-03-17 1316 if (cstate.period > 0 &&
0835e5c8704297 Uwe Kleine-König 2024-03-17 1317 (cstate.duty_cycle > cstate.period ||
0835e5c8704297 Uwe Kleine-König 2024-03-17 1318 cstate.duty_offset >= cstate.period)) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1319 ret = -EINVAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1320 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1321 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1322
0835e5c8704297 Uwe Kleine-König 2024-03-17 1323 /*
0835e5c8704297 Uwe Kleine-König 2024-03-17 1324 * While the API provides a duty_offset member
0835e5c8704297 Uwe Kleine-König 2024-03-17 1325 * to describe (among others) also inversed
0835e5c8704297 Uwe Kleine-König 2024-03-17 1326 * polarity wave forms, the translation into the
0835e5c8704297 Uwe Kleine-König 2024-03-17 1327 * traditional representation with a (binary) polarity
0835e5c8704297 Uwe Kleine-König 2024-03-17 1328 * isn't trivial because the lowlevel drivers round
0835e5c8704297 Uwe Kleine-König 2024-03-17 1329 * duty_cycle down when applying a setting and so in the
0835e5c8704297 Uwe Kleine-König 2024-03-17 1330 * representation with duty_offset the rounding is
0835e5c8704297 Uwe Kleine-König 2024-03-17 1331 * inconsistent. I have no idea what's the best way to
0835e5c8704297 Uwe Kleine-König 2024-03-17 1332 * fix that, so to not commit to a solution yet, just
0835e5c8704297 Uwe Kleine-König 2024-03-17 1333 * refuse requests with .duty_offset that would yield
0835e5c8704297 Uwe Kleine-König 2024-03-17 1334 * inversed polarity for now.
0835e5c8704297 Uwe Kleine-König 2024-03-17 1335 */
0835e5c8704297 Uwe Kleine-König 2024-03-17 1336 if (cstate.duty_cycle < cstate.period &&
0835e5c8704297 Uwe Kleine-König 2024-03-17 1337 cstate.duty_offset + cstate.duty_cycle >= cstate.period) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1338 ret = -EINVAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1339 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1340 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1341
0835e5c8704297 Uwe Kleine-König 2024-03-17 1342 ret = pwm_cdev_request(cdata, cstate.hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1343 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1344 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1345
0835e5c8704297 Uwe Kleine-König 2024-03-17 1346 pwm = cdata->pwm[cstate.hwpwm];
0835e5c8704297 Uwe Kleine-König 2024-03-17 1347
0835e5c8704297 Uwe Kleine-König 2024-03-17 1348 if (cstate.period > 0) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1349 state.enabled = true;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1350 state.period = cstate.period;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1351 state.polarity = PWM_POLARITY_NORMAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1352 state.duty_cycle = cstate.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1353 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1354 state.enabled = false;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1355 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1356
0835e5c8704297 Uwe Kleine-König 2024-03-17 1357 ret = pwm_apply_might_sleep(pwm, &state);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1358 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1359 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1360
0835e5c8704297 Uwe Kleine-König 2024-03-17 1361 default:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1362 ret = -ENOTTY;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1363 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1364 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1365 out_unlock:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1366 mutex_unlock(&pwm_lock);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1367
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1368 return ret;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1369 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1370
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-next:master 1131/2532] drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
2024-04-03 10:28 ` Uwe Kleine-König
@ 2024-04-03 10:48 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2024-04-03 10:48 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: oe-kbuild, lkp, oe-kbuild-all
On Wed, Apr 03, 2024 at 12:28:57PM +0200, Uwe Kleine-König wrote:
> On Tue, Apr 02, 2024 at 05:26:31PM +0300, Dan Carpenter wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
> > commit: 0835e5c8704297e91b54071c3bbcd0c05c63bd3f [1131/2532] pwm: Add support for pwmchip devices for faster and easier userspace access
> > config: m68k-randconfig-r071-20240326 (https://download.01.org/0day-ci/archive/20240401/202404010703.o1yYkXrA-lkp@intel.com/config)
> > compiler: m68k-linux-gcc (GCC) 13.2.0
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202404010703.o1yYkXrA-lkp@intel.com/
> >
> > New smatch warnings:
> > drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
> > drivers/pwm/core.c:1284 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
> > drivers/pwm/core.c:1368 pwm_cdev_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?
>
> I fixed the issue in line 1368 in my pwm/for-next branch.
>
> For the spectre issue my first guess is:
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 2745941a008b..dd54acd336a6 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -17,6 +17,7 @@
> #include <linux/slab.h>
> #include <linux/device.h>
> #include <linux/debugfs.h>
> +#include <linux/nospec.h>
> #include <linux/seq_file.h>
>
> #include <dt-bindings/pwm/pwm.h>
> @@ -1191,6 +1192,12 @@ static int pwm_cdev_request(struct pwm_cdev_data *cdata, unsigned int hwpwm)
> if (hwpwm >= chip->npwm)
> return -EINVAL;
>
> + /*
> + * Mitigate speculation side-channels; without considering speculation
> + * this is a noop.
> + */
> + hwpwm = array_index_nospec(hwpwm, chip->npwm);
> +
> if (!cdata->pwm[hwpwm]) {
> struct pwm_device *pwm = &chip->pwms[hwpwm];
> int ret;
>
> But I'm unsure if this also addresses the issue for
>
> pwm = cdata->pwm[cstate.hwpwm];
>
> Maybe pwm_cdev_request() is better changed to return the struct
> pwm_device *? Who is the expert, who can help here?
I believe that your patch does fix it. I'm not sure who the experts are
but probably they are on the x86@kernel.org mailing list.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-next:master 1131/2532] drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
2024-04-02 14:26 Dan Carpenter
@ 2024-04-03 10:28 ` Uwe Kleine-König
2024-04-03 10:48 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2024-04-03 10:28 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, lkp, oe-kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]
On Tue, Apr 02, 2024 at 05:26:31PM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
> commit: 0835e5c8704297e91b54071c3bbcd0c05c63bd3f [1131/2532] pwm: Add support for pwmchip devices for faster and easier userspace access
> config: m68k-randconfig-r071-20240326 (https://download.01.org/0day-ci/archive/20240401/202404010703.o1yYkXrA-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202404010703.o1yYkXrA-lkp@intel.com/
>
> New smatch warnings:
> drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
> drivers/pwm/core.c:1284 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
> drivers/pwm/core.c:1368 pwm_cdev_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?
I fixed the issue in line 1368 in my pwm/for-next branch.
For the spectre issue my first guess is:
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 2745941a008b..dd54acd336a6 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/device.h>
#include <linux/debugfs.h>
+#include <linux/nospec.h>
#include <linux/seq_file.h>
#include <dt-bindings/pwm/pwm.h>
@@ -1191,6 +1192,12 @@ static int pwm_cdev_request(struct pwm_cdev_data *cdata, unsigned int hwpwm)
if (hwpwm >= chip->npwm)
return -EINVAL;
+ /*
+ * Mitigate speculation side-channels; without considering speculation
+ * this is a noop.
+ */
+ hwpwm = array_index_nospec(hwpwm, chip->npwm);
+
if (!cdata->pwm[hwpwm]) {
struct pwm_device *pwm = &chip->pwms[hwpwm];
int ret;
But I'm unsure if this also addresses the issue for
pwm = cdata->pwm[cstate.hwpwm];
Maybe pwm_cdev_request() is better changed to return the struct
pwm_device *? Who is the expert, who can help here?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [linux-next:master 1131/2532] drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
@ 2024-04-02 14:26 Dan Carpenter
2024-04-03 10:28 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2024-04-02 14:26 UTC (permalink / raw)
To: oe-kbuild, Uwe Kleine-König; +Cc: lkp, oe-kbuild-all
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
commit: 0835e5c8704297e91b54071c3bbcd0c05c63bd3f [1131/2532] pwm: Add support for pwmchip devices for faster and easier userspace access
config: m68k-randconfig-r071-20240326 (https://download.01.org/0day-ci/archive/20240401/202404010703.o1yYkXrA-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202404010703.o1yYkXrA-lkp@intel.com/
New smatch warnings:
drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r]
drivers/pwm/core.c:1284 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
drivers/pwm/core.c:1368 pwm_cdev_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?
Old smatch warnings:
drivers/pwm/core.c:1357 pwm_cdev_ioctl() warn: possible spectre second half. 'pwm'
drivers/pwm/core.c:1871 pwm_put() warn: variable dereferenced before check 'pwm' (see line 1869)
vim +1282 drivers/pwm/core.c
0835e5c8704297 Uwe Kleine-König 2024-03-17 1226 static long pwm_cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1227 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1228 int ret = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1229 struct pwm_cdev_data *cdata = file->private_data;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1230 struct pwm_chip *chip = cdata->chip;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1231
0835e5c8704297 Uwe Kleine-König 2024-03-17 1232 mutex_lock(&pwm_lock);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1233
0835e5c8704297 Uwe Kleine-König 2024-03-17 1234 if (!chip->operational) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1235 ret = -ENODEV;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1236 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1237 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1238
0835e5c8704297 Uwe Kleine-König 2024-03-17 1239 switch (cmd) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1240 case PWM_IOCTL_GET_NUM_PWMS:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1241 ret = chip->npwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1242 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1243
0835e5c8704297 Uwe Kleine-König 2024-03-17 1244 case PWM_IOCTL_REQUEST:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1245 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1246 unsigned int hwpwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1247
0835e5c8704297 Uwe Kleine-König 2024-03-17 1248 ret = get_user(hwpwm, (unsigned int __user *)arg);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1249 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1250 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1251
0835e5c8704297 Uwe Kleine-König 2024-03-17 1252 ret = pwm_cdev_request(cdata, hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1253 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1254 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1255
0835e5c8704297 Uwe Kleine-König 2024-03-17 1256 case PWM_IOCTL_FREE:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1257 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1258 unsigned int hwpwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1259
0835e5c8704297 Uwe Kleine-König 2024-03-17 1260 ret = get_user(hwpwm, (unsigned int __user *)arg);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1261 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1262 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1263
0835e5c8704297 Uwe Kleine-König 2024-03-17 1264 ret = pwm_cdev_free(cdata, hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1265 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1266 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1267
0835e5c8704297 Uwe Kleine-König 2024-03-17 1268 case PWM_IOCTL_GET:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1269 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1270 struct pwmchip_state cstate;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1271 struct pwm_state state;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1272 struct pwm_device *pwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1273
0835e5c8704297 Uwe Kleine-König 2024-03-17 1274 ret = copy_from_user(&cstate, (struct pwmchip_state __user *)arg, sizeof(cstate));
if (copy_from_user(&cstate, (struct pwmchip_state __user *)arg, sizeof(cstate))) {
ret = -EFAULT;
goto out_unlock;
}
0835e5c8704297 Uwe Kleine-König 2024-03-17 1275 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1276 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1277
0835e5c8704297 Uwe Kleine-König 2024-03-17 1278 ret = pwm_cdev_request(cdata, cstate.hwpwm);
I don't understand very much about spectre but I think maybe there is
a speculation issue inside pwm_cdev_request() where we need to add an
array_index_nospec().
0835e5c8704297 Uwe Kleine-König 2024-03-17 1279 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1280 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1281
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1282 pwm = cdata->pwm[cstate.hwpwm];
0835e5c8704297 Uwe Kleine-König 2024-03-17 1283
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1284 ret = pwm_get_state_hw(pwm, &state);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1285 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1286 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1287
0835e5c8704297 Uwe Kleine-König 2024-03-17 1288 if (state.enabled) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1289 cstate.period = state.period;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1290 if (state.polarity == PWM_POLARITY_NORMAL) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1291 cstate.duty_offset = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1292 cstate.duty_cycle = state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1293 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1294 cstate.duty_offset = state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1295 cstate.duty_cycle = state.period - state.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1296 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1297 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1298 cstate.period = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1299 cstate.duty_cycle = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1300 cstate.duty_offset = 0;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1301 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1302 ret = copy_to_user((struct pwmchip_state __user *)arg, &cstate, sizeof(cstate));
^^^^^^^^^^^^^^^^^^^
0835e5c8704297 Uwe Kleine-König 2024-03-17 1303 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1304 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1305
0835e5c8704297 Uwe Kleine-König 2024-03-17 1306 case PWM_IOCTL_APPLY:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1307 {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1308 struct pwmchip_state cstate;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1309 struct pwm_state state = { };
0835e5c8704297 Uwe Kleine-König 2024-03-17 1310 struct pwm_device *pwm;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1311
0835e5c8704297 Uwe Kleine-König 2024-03-17 1312 ret = copy_from_user(&cstate, (struct pwmchip_state __user *)arg, sizeof(cstate));
^^^^^^^^^^^^^^^^^^^^
0835e5c8704297 Uwe Kleine-König 2024-03-17 1313 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1314 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1315
0835e5c8704297 Uwe Kleine-König 2024-03-17 1316 if (cstate.period > 0 &&
0835e5c8704297 Uwe Kleine-König 2024-03-17 1317 (cstate.duty_cycle > cstate.period ||
0835e5c8704297 Uwe Kleine-König 2024-03-17 1318 cstate.duty_offset >= cstate.period)) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1319 ret = -EINVAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1320 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1321 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1322
0835e5c8704297 Uwe Kleine-König 2024-03-17 1323 /*
0835e5c8704297 Uwe Kleine-König 2024-03-17 1324 * While the API provides a duty_offset member
0835e5c8704297 Uwe Kleine-König 2024-03-17 1325 * to describe (among others) also inversed
0835e5c8704297 Uwe Kleine-König 2024-03-17 1326 * polarity wave forms, the translation into the
0835e5c8704297 Uwe Kleine-König 2024-03-17 1327 * traditional representation with a (binary) polarity
0835e5c8704297 Uwe Kleine-König 2024-03-17 1328 * isn't trivial because the lowlevel drivers round
0835e5c8704297 Uwe Kleine-König 2024-03-17 1329 * duty_cycle down when applying a setting and so in the
0835e5c8704297 Uwe Kleine-König 2024-03-17 1330 * representation with duty_offset the rounding is
0835e5c8704297 Uwe Kleine-König 2024-03-17 1331 * inconsistent. I have no idea what's the best way to
0835e5c8704297 Uwe Kleine-König 2024-03-17 1332 * fix that, so to not commit to a solution yet, just
0835e5c8704297 Uwe Kleine-König 2024-03-17 1333 * refuse requests with .duty_offset that would yield
0835e5c8704297 Uwe Kleine-König 2024-03-17 1334 * inversed polarity for now.
0835e5c8704297 Uwe Kleine-König 2024-03-17 1335 */
0835e5c8704297 Uwe Kleine-König 2024-03-17 1336 if (cstate.duty_cycle < cstate.period &&
0835e5c8704297 Uwe Kleine-König 2024-03-17 1337 cstate.duty_offset + cstate.duty_cycle >= cstate.period) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1338 ret = -EINVAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1339 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1340 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1341
0835e5c8704297 Uwe Kleine-König 2024-03-17 1342 ret = pwm_cdev_request(cdata, cstate.hwpwm);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1343 if (ret)
0835e5c8704297 Uwe Kleine-König 2024-03-17 1344 goto out_unlock;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1345
0835e5c8704297 Uwe Kleine-König 2024-03-17 1346 pwm = cdata->pwm[cstate.hwpwm];
0835e5c8704297 Uwe Kleine-König 2024-03-17 1347
0835e5c8704297 Uwe Kleine-König 2024-03-17 1348 if (cstate.period > 0) {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1349 state.enabled = true;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1350 state.period = cstate.period;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1351 state.polarity = PWM_POLARITY_NORMAL;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1352 state.duty_cycle = cstate.duty_cycle;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1353 } else {
0835e5c8704297 Uwe Kleine-König 2024-03-17 1354 state.enabled = false;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1355 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1356
0835e5c8704297 Uwe Kleine-König 2024-03-17 1357 ret = pwm_apply_might_sleep(pwm, &state);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1358 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1359 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1360
0835e5c8704297 Uwe Kleine-König 2024-03-17 1361 default:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1362 ret = -ENOTTY;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1363 break;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1364 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1365 out_unlock:
0835e5c8704297 Uwe Kleine-König 2024-03-17 1366 mutex_unlock(&pwm_lock);
0835e5c8704297 Uwe Kleine-König 2024-03-17 1367
0835e5c8704297 Uwe Kleine-König 2024-03-17 @1368 return ret;
0835e5c8704297 Uwe Kleine-König 2024-03-17 1369 }
0835e5c8704297 Uwe Kleine-König 2024-03-17 1370
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-03 10:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-31 23:09 [linux-next:master 1131/2532] drivers/pwm/core.c:1282 pwm_cdev_ioctl() warn: potential spectre issue 'cdata->pwm' [r] kernel test robot
2024-04-02 14:26 Dan Carpenter
2024-04-03 10:28 ` Uwe Kleine-König
2024-04-03 10:48 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).