| Message ID | 1614580437-19660-1-git-send-email-wangqing@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arch: mips: sibyte: Return -EFAULT if copy_to_user() fails | expand |
Hello! On 01.03.2021 9:33, Wang Qing wrote: > The copy_to_user() function returns the number of bytes remaining to be > copied, but we want to return -EFAULT if the copy doesn't complete. Then 'err' is hardly a good name for that variable. :-) > > Signed-off-by: Wang Qing <wangqing@vivo.com> > --- > arch/mips/sibyte/common/sb_tbprof.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/mips/sibyte/common/sb_tbprof.c b/arch/mips/sibyte/common/sb_tbprof.c > index f80d7a7..eac125f > --- a/arch/mips/sibyte/common/sb_tbprof.c > +++ b/arch/mips/sibyte/common/sb_tbprof.c > @@ -465,7 +465,7 @@ static ssize_t sbprof_tb_read(struct file *filp, char *buf, > if (err) { > *offp = cur_off + cur_count - err; > mutex_unlock(&sbp.lock); > - return err; > + return -EFAULT; > } > pr_debug(DEVNAME ": read from sample %d, %d bytes\n", > cur_sample, cur_count); MBR, Sergei
On Mon, 1 Mar 2021, Sergei Shtylyov wrote: > > The copy_to_user() function returns the number of bytes remaining to be > > copied, but we want to return -EFAULT if the copy doesn't complete. > > Then 'err' is hardly a good name for that variable. :-) Something like `left' might be better, especially as it's the sole use. Also it's been like this from the beginning, so: Fixes: bb9b813bb665 ("[MIPS] Sibyte: Fix ZBbus profiler") or maybe: Fixes: d619f38fdacb ("[MIPS] Add bcm1480 ZBus trace support, fix wait related bugs") (since the file was renamed from a different name with the latter commit) would I think be helpful for backports too. It looks like potentially quite a nasty bug to me if someone actually uses this piece (I haven't). Maciej
diff --git a/arch/mips/sibyte/common/sb_tbprof.c b/arch/mips/sibyte/common/sb_tbprof.c index f80d7a7..eac125f --- a/arch/mips/sibyte/common/sb_tbprof.c +++ b/arch/mips/sibyte/common/sb_tbprof.c @@ -465,7 +465,7 @@ static ssize_t sbprof_tb_read(struct file *filp, char *buf, if (err) { *offp = cur_off + cur_count - err; mutex_unlock(&sbp.lock); - return err; + return -EFAULT; } pr_debug(DEVNAME ": read from sample %d, %d bytes\n", cur_sample, cur_count);
The copy_to_user() function returns the number of bytes remaining to be copied, but we want to return -EFAULT if the copy doesn't complete. Signed-off-by: Wang Qing <wangqing@vivo.com> --- arch/mips/sibyte/common/sb_tbprof.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)