On Thu, 2021-02-11 at 10:11 +0300, Dan Carpenter wrote: > On Wed, Feb 10, 2021 at 02:28:54PM -0800, Drew Fustini wrote: > > + ret = strncpy_from_user(buf, user_buf, PINMUX_MAX_NAME * 2); > > + 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); > > + if (ret != 2) { > > + dev_err(pctldev->dev, "expected format: <function-name> <group-name>"); > > + goto free_gname; > > We need a "ret = -EINVAL;" before the goto. sscanf doesn't return error > codes. Normally we would write it like so: > > if (sscanf(buf, "%s %s", fname, gname) != 2) { > dev_err(pctldev->dev, "expected format: <function-name> <group-name>"); > ret = -EINVAL; > goto free_gname; > } > > I'm going to write a Smatch check for this today. It's a pretty frequently used style: $ git grep -P '\w+\s*=\s+sscanf\b' | wc -l 327 A grep with -A5 seems to show most use some additional error assignment when checking the return value. $ git grep -P -A5 '\w+\s*=\s+sscanf\b' | grep -P '(?:return|=)\s*\-E' | wc -l 174