On Thu, Feb 11, 2021 at 09:09:03AM +0100, Geert Uytterhoeven wrote:

Hi Drew,

On Wed, Feb 10, 2021 at 11:33 PM Drew Fustini [off-list ref] wrote:

quoted

Add "pinmux-select" to debugfs which will activate a function and group
when "<function-name group-name>" are written to the file. The write
operation pinmux_select() handles this by checking that the names map to
valid selectors and then calling ops->set_mux().

The existing "pinmux-functions" debugfs file lists the pin functions
registered for the pin controller. For example:

function: pinmux-uart0, groups = [ pinmux-uart0-pins ]
function: pinmux-mmc0, groups = [ pinmux-mmc0-pins ]
function: pinmux-mmc1, groups = [ pinmux-mmc1-pins ]
function: pinmux-i2c0, groups = [ pinmux-i2c0-pins ]
function: pinmux-i2c1, groups = [ pinmux-i2c1-pins ]
function: pinmux-spi1, groups = [ pinmux-spi1-pins ]

To activate function pinmux-i2c1 and group pinmux-i2c1-pins:

echo "pinmux-i2c1 pinmux-i2c1-pins" > pinmux-select

Signed-off-by: Drew Fustini <redacted>

Thanks for your patch!

quoted

--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c

@@ -673,6 +673,111 @@ void pinmux_show_setting(struct seq_file *s,
 DEFINE_SHOW_ATTRIBUTE(pinmux_functions);
 DEFINE_SHOW_ATTRIBUTE(pinmux_pins);

+#define PINMUX_MAX_NAME 64
+static ssize_t pinmux_select(struct file *file, const char __user *user_buf,
+                                  size_t len, loff_t *ppos)
+{
+       struct seq_file *sfile = file->private_data;
+       struct pinctrl_dev *pctldev = sfile->private;
+       const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+       const char *const *groups;
+       char *buf, *fname, *gname;
+       unsigned int num_groups;
+       int fsel, gsel, ret;
+
+       if (len > (PINMUX_MAX_NAME * 2)) {
+               dev_err(pctldev->dev, "write too big for buffer");
+               return -EINVAL;
+       }
+
+       buf = kzalloc(PINMUX_MAX_NAME * 2, GFP_KERNEL);
+       if (!buf)
+               return -ENOMEM;
+
+       fname = kzalloc(PINMUX_MAX_NAME, GFP_KERNEL);
+       if (!fname) {
+               ret = -ENOMEM;
+               goto free_buf;
+       }
+
+       gname = kzalloc(PINMUX_MAX_NAME, GFP_KERNEL);
+       if (!buf) {
+               ret = -ENOMEM;
+               goto free_fname;
+       }
+
+       ret = strncpy_from_user(buf, user_buf, PINMUX_MAX_NAME * 2);

While this guarantees buf is not overflowed...

quoted

+       if (ret < 0) {
+               dev_err(pctldev->dev, "failed to copy buffer from userspace");
+               goto free_gname;
+       }
+       buf[len-1] = '\0';
+
+       ret = sscanf(buf, "%s %s", fname, gname);

... one of the two strings can still be longer than PINMUX_MAX_NAME,
thus overflowing fname or gname.

As buf is already a copy, it may be easier to just find the strings in
buf, write the NUL terminators into buf, and set up fname and gname
to point to the strings inside buf.

Thank you for pointing this out.  I should have considered that one name
could be much larger than the other name.  I see Andy suggested
alternative to sscanf() that gets around having to allocate seperate
buffers for each name.

quoted

+       if (ret != 2) {
+               dev_err(pctldev->dev, "expected format: <function-name> <group-name>");
+               goto free_gname;
+       }

quoted

+static const struct file_operations pinmux_select_ops = {
+       .owner = THIS_MODULE,
+       .open = pinmux_select_open,
+       .read = seq_read,

I don't think you need to fill in .read for a write-only file.

Thanks, I'll remove it.

quoted

+       .write = pinmux_select,
+       .llseek = no_llseek,
+       .release = single_release,
+};
+
 void pinmux_init_device_debugfs(struct dentry *devroot,
                         struct pinctrl_dev *pctldev)
 {

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds