pinctrl: Duplicate user memory in one go in pinmux_select()
Subsystems (3)
PATCH 1/2 pinctrl: Duplicate user memory in one go in pinmux_select()
Current code is suboptimal in three ways:
1) it explicitly terminates the string which is not needed;
2) it might provoke additional faults, because asked lenght might be bigger than the real one;
3) it consumes more than needed lines in the source.
Instead of using kmalloc() + strncpy_from_user() + terminating, just utilize memdup_user_nul().
--- drivers/pinctrl/pinmux.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/pinctrl/pinmux.c +3 -8
@@ -692,14 +692,9 @@ static ssize_t pinmux_select(struct file *file, const char __user *user_buf,if (len >PINMUX_SELECT_MAX )return -ENOMEM ; - buf =kzalloc (PINMUX_SELECT_MAX ,GFP_KERNEL ); -if (!buf) -return -ENOMEM ; - - ret =strncpy_from_user (buf, user_buf,PINMUX_SELECT_MAX ); -if (ret <0 ) - goto exit_free_buf; - buf[len-1 ] ='\0' ; + buf =memdup_user_nul (user_buf, len); +if (IS_ERR (buf)) +return PTR_ERR (buf);/* remove leading and trailing spaces of input buffer */ gname =strstrip (buf);
Signature · 1 line
2.40.0.1.gaa8946217a0b
PATCH 2/2 pinctrl: Relax user input size in pinmux_select()
This is debugfs and there is no much sense to strict the user from sending as much data as they can. The memdup_user_nul() will anyway fail if there is not enough memory.
Relax the user input size by removing an artificial limitaion.
--- drivers/pinctrl/pinmux.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/pinctrl/pinmux.c +0 -4
@@ -677,7 +677,6 @@ void pinmux_show_setting(struct seq_file *s,DEFINE_SHOW_ATTRIBUTE (pinmux_functions);DEFINE_SHOW_ATTRIBUTE (pinmux_pins); -#define PINMUX_SELECT_MAX 128static ssize_t pinmux_select (struct file *file,const char __user *user_buf,size_t len,loff_t *ppos) {
@@ -689,9 +688,6 @@ static ssize_t pinmux_select(struct file *file, const char __user *user_buf,unsigned int num_groups;int fsel, gsel, ret; -if (len >PINMUX_SELECT_MAX ) -return -ENOMEM ; - buf =memdup_user_nul (user_buf, len);if (IS_ERR (buf))return PTR_ERR (buf);
Signature · 1 line
2.40.0.1.gaa8946217a0b
Re: [PATCH v1 2/2] pinctrl: Relax user input size in pinmux_select()
Quoted 7 lines · Andy Shevchenko
This is debugfs and there is no much sense to strict the user from sending as much data as they can. The
memdup_user_nul()will anyway fail if there is not enough memory.Relax the user input size by removing an artificial limitaion.
Signed-off-by Andy Shevchenko <[email protected]>
Fair enough, patch applied!
Yours,
Linus Walleij
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 2/2] pinctrl: Relax user input size in pinmux_select()
Quoted 7 lines · Markus Elfring
Quoted 5 lines
This is debugfs and there is no much sense to strict the user from sending as much data as they can. The
memdup_user_nul()will anyway fail if there is not enough memory.Relax the user input size by removing an artificial limitaion.
How are the chances to avoid typos also in such a change description?
Almost 0. This is now part of the non-rebased branch. It can be fixed if and only if
- something really wrong happened before this patch in the tree and has to be fixed, and
- maintainer will remember to fix the typo.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 2/2] pinctrl: Relax user input size in pinmux_select()
In reply to a message not in the archiveQuoted 7 lines · Markus Elfring
Quoted 5 lines
This is debugfs and there is no much sense to strict the user from sending as much data as they can. The
memdup_user_nul()will anyway fail if there is not enough memory.Relax the user input size by removing an artificial limitaion.
How are the chances to avoid typos also in such a change description?
I don't care much about typos, and when a patch is coming from Andy it's certainly not a blocker for anything since his patches are of high technical value.
Yours,
Linus Walleij
Quoted 10 lines · Andy Shevchenko
Current code is suboptimal in three ways:
1) it explicitly terminates the string which is not needed;
2) it might provoke additional faults, because asked lenght might be bigger than the real one;
3) it consumes more than needed lines in the source.Instead of using
kmalloc()+strncpy_from_user()+ terminating, just utilizememdup_user_nul().Signed-off-by Andy Shevchenko <[email protected]>
Excellent patch, applied!
Yours,
Linus Walleij
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 3 lines · Markus Elfring
Quoted 1 line
…, because asked lenght …
* Are there any chances to avoid a typo in such a change description?
Not anymore as explained in the other email, but thanks for pointing this out.
Quoted 1 line
* Was a cover letter accidentally omitted?
What do you want to see in such cover letter? How can it be helpful? (Note that these are rhetorical towards this change, as it's already
in the non-rebased branch)
Signature · 2 lines
With Best Regards, Andy Shevchenko