Re: [PATCH] cxl: Fix error path on bad ioctl
Frederic Barratwrites: > Le 06/06/2017 à 11:20, Michael Ellerman a écrit : >> Frederic Barrat writes: >> >>> Fix error path if we can't copy user structure on >>> CXL_IOCTL_START_WORK ioctl. >> >> To be clear the error is that returning via the out label will unlock >> cxl->status_mutex, which has not been locked. >> >> Please spell it out for me :) >> >> This should be: >> >>Fixes: 0712dc7e73e5 ("cxl: Fix issues when unmapping contexts") >> >> Am I right? > > That's correct. I'm about to send a v2 to address Vaibhav's comment and > I'll fix the above as well. Thanks. cheers
Re: [PATCH] cxl: Fix error path on bad ioctl
Le 06/06/2017 à 11:20, Michael Ellerman a écrit : Frederic Barratwrites: Fix error path if we can't copy user structure on CXL_IOCTL_START_WORK ioctl. To be clear the error is that returning via the out label will unlock cxl->status_mutex, which has not been locked. Please spell it out for me :) This should be: Fixes: 0712dc7e73e5 ("cxl: Fix issues when unmapping contexts") Am I right? That's correct. I'm about to send a v2 to address Vaibhav's comment and I'll fix the above as well. Thanks, Fred cheers diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c index 17b433f1ce23..caa44adfa60e 100644 --- a/drivers/misc/cxl/file.c +++ b/drivers/misc/cxl/file.c @@ -160,10 +160,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx, /* Do this outside the status_mutex to avoid a circular dependency with * the locking in cxl_mmap_fault() */ if (copy_from_user(, uwork, - sizeof(struct cxl_ioctl_start_work))) { - rc = -EFAULT; - goto out; - } + sizeof(struct cxl_ioctl_start_work))) + return -EFAULT; mutex_lock(>status_mutex); if (ctx->status != OPENED) { -- 2.11.0
Re: [PATCH] cxl: Fix error path on bad ioctl
Frederic Barratwrites: > Fix error path if we can't copy user structure on > CXL_IOCTL_START_WORK ioctl. To be clear the error is that returning via the out label will unlock cxl->status_mutex, which has not been locked. Please spell it out for me :) This should be: Fixes: 0712dc7e73e5 ("cxl: Fix issues when unmapping contexts") Am I right? cheers > diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c > index 17b433f1ce23..caa44adfa60e 100644 > --- a/drivers/misc/cxl/file.c > +++ b/drivers/misc/cxl/file.c > @@ -160,10 +160,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx, > /* Do this outside the status_mutex to avoid a circular dependency with >* the locking in cxl_mmap_fault() */ > if (copy_from_user(, uwork, > -sizeof(struct cxl_ioctl_start_work))) { > - rc = -EFAULT; > - goto out; > - } > +sizeof(struct cxl_ioctl_start_work))) > + return -EFAULT; > > mutex_lock(>status_mutex); > if (ctx->status != OPENED) { > -- > 2.11.0
Re: [PATCH] cxl: Fix error path on bad ioctl
On 03/06/17 02:15, Frederic Barrat wrote: Fix error path if we can't copy user structure on CXL_IOCTL_START_WORK ioctl. Signed-off-by: Frederic BarratCc: sta...@vger.kernel.org Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: [PATCH] cxl: Fix error path on bad ioctl
Hi Fred, Good catch. Frederic Barratwrites: > Fix error path if we can't copy user structure on > CXL_IOCTL_START_WORK ioctl. > > Signed-off-by: Frederic Barrat > Cc: sta...@vger.kernel.org > --- > drivers/misc/cxl/file.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c > index 17b433f1ce23..caa44adfa60e 100644 > --- a/drivers/misc/cxl/file.c > +++ b/drivers/misc/cxl/file.c > @@ -160,10 +160,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx, > /* Do this outside the status_mutex to avoid a circular dependency with >* the locking in cxl_mmap_fault() */ > if (copy_from_user(, uwork, > -sizeof(struct cxl_ioctl_start_work))) { > - rc = -EFAULT; > - goto out; > - } > +sizeof(struct cxl_ioctl_start_work))) Bike-shedding a bit, but s/sizeof(struct cxl_ioctl_start_work)))/sizeof(work)/ would look much cleaner Reviewed-by: Vaibhav Jain
[PATCH] cxl: Fix error path on bad ioctl
Fix error path if we can't copy user structure on CXL_IOCTL_START_WORK ioctl. Signed-off-by: Frederic BarratCc: sta...@vger.kernel.org --- drivers/misc/cxl/file.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c index 17b433f1ce23..caa44adfa60e 100644 --- a/drivers/misc/cxl/file.c +++ b/drivers/misc/cxl/file.c @@ -160,10 +160,8 @@ static long afu_ioctl_start_work(struct cxl_context *ctx, /* Do this outside the status_mutex to avoid a circular dependency with * the locking in cxl_mmap_fault() */ if (copy_from_user(, uwork, - sizeof(struct cxl_ioctl_start_work))) { - rc = -EFAULT; - goto out; - } + sizeof(struct cxl_ioctl_start_work))) + return -EFAULT; mutex_lock(>status_mutex); if (ctx->status != OPENED) { -- 2.11.0