On Tue, Oct 05, 2021 at 09:30:10AM -0700, Kees Cook wrote:
On Mon, Sep 27, 2021 at 09:37:56AM -0700, Luis Chamberlain wrote:
--- /dev/null +++ b/lib/test_sysfs.c @@ -0,0 +1,921 @@ +// SPDX-License-Identifier: GPL-2.0-or-later OR copyleft-next-0.3.1 +/*
- sysfs test driver
- Copyright (C) 2021 Luis Chamberlain mcgrof@kernel.org
- This program is free software; you can redistribute it and/or modify it
- under the terms of the GNU General Public License as published by the Free
- Software Foundation; either version 2 of the License, or at your option any
- later version; or, when distributed separately from the Linux kernel or
- when incorporated into other software packages, subject to the following
- license:
- This program is free software; you can redistribute it and/or modify it
- under the terms of copyleft-next (version 0.3.1 or later) as published
As Greg suggested, please drop the boilerplate here.
Sure, sorry for missing that fixed.
+static ssize_t config_show(struct device *dev,
struct device_attribute *attr,char *buf)+{
- struct sysfs_test_device *test_dev = dev_to_test_dev(dev);
- struct test_config *config = &test_dev->config;
- int len = 0;
- test_dev_config_lock(test_dev);
- len += snprintf(buf, PAGE_SIZE,
"Configuration for: %s\n",dev_name(dev));Please use sysfs_emit() instead of snprintf().
Oh nice, done and fixed also in the other places.
+static int sysfs_test_dev_alloc_blockdev(struct sysfs_test_device *test_dev) +{
- int ret = -ENOMEM;
- test_dev->disk = blk_alloc_disk(NUMA_NO_NODE);
- if (!test_dev->disk) {
pr_err("Error allocating disk structure for device %d\n",test_dev->dev_idx);goto out;- }
- test_dev->disk->major = sysfs_test_major;
- test_dev->disk->first_minor = test_dev->dev_idx + 1;
- test_dev->disk->fops = &sysfs_testdev_ops;
- test_dev->disk->private_data = test_dev;
- snprintf(test_dev->disk->disk_name, 16, "test_sysfs%d",
test_dev->dev_idx);Prefer sizeof(test_dev->disk->disk_name) over open-coded "16".
Sure.
+static ssize_t read_reset_first_test_dev(struct file *file,
char __user *user_buf,size_t count, loff_t *ppos)+{
- ssize_t len;
- char buf[32];
- reset_first_test_dev++;
- len = sprintf(buf, "%d\n", reset_first_test_dev);
Even though it's safe as-is, I was going to suggest scnprintf() here (i.e. explicit bounds and a bounds-checked "len"). However, scnprintf() returns ssize_t, and there's no bounds checking in simple_read_from_buffer. That needs fixing (I'll send a patch).
OK we can later change it to scnprintf() once your patch gets merged.
--- /dev/null +++ b/tools/testing/selftests/sysfs/sysfs.sh @@ -0,0 +1,1208 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2021 Luis Chamberlain mcgrof@kernel.org +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 2 of the License, or at your option any +# later version; or, when distributed separately from the Linux kernel or +# when incorporated into other software packages, subject to the following +# license: +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of copyleft-next (version 0.3.1 or later) as published +# at http://copyleft-next.org/.
+# This performs a series tests against the sysfs filesystem.
-boilerplate
Nuked.
+check_dmesg() +{
- # filter out intentional WARNINGs or Oopses
- local filter=${1:-_check_dmesg_filter}
- _dmesg_since_test_start | $filter >$seqres.dmesg
- egrep -q -e "kernel BUG at" \
-e "WARNING:" \-e "\bBUG:" \-e "Oops:" \-e "possible recursive locking detected" \-e "Internal error" \-e "(INFO|ERR): suspicious RCU usage" \-e "INFO: possible circular locking dependency detected" \-e "general protection fault:" \-e "BUG .* remaining" \-e "UBSAN:" \$seqres.dmesgIs just looking for "call trace" sufficient here?
So far from my testing yes. This strategy is also borrowed from fstests and that's what is done there, and so quite a lot of testing has been done with that. If we are to consider an enhancement here we should then also consider an enhancement welcome for fstests.
Luis