| Message ID | 20211015050345.GA1161918@embeddedor (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36 |
| Delegated to: | Gustavo A. R. Silva |
| Headers | show |
| Series | [next] powerpc/vas: Fix potential NULL pointer dereference | expand |
On 10/14/21 10:03 PM, Gustavo A. R. Silva wrote: > (!ptr && !ptr->foo) strikes again. :) > > The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, > it leads to a NULL pointer dereference: ptr->foo. > > Fix this by converting && to || > > This issue was detected with the help of Coccinelle, and audited and > fixed manually. > > Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window operations") > Cc: stable@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looking at the usage pattern it is obvious that if we determine !ptr attempting to also confirm !ptr->ops is going to blow up. LGTM. Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com> > --- > arch/powerpc/platforms/book3s/vas-api.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c > index 30172e52e16b..4d82c92ddd52 100644 > --- a/arch/powerpc/platforms/book3s/vas-api.c > +++ b/arch/powerpc/platforms/book3s/vas-api.c > @@ -303,7 +303,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg) > return -EINVAL; > } > > - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->open_win) { > + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->open_win) { > pr_err("VAS API is not registered\n"); > return -EACCES; > } > @@ -373,7 +373,7 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma) > return -EINVAL; > } > > - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->paste_addr) { > + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->paste_addr) { > pr_err("%s(): VAS API is not registered\n", __func__); > return -EACCES; > } >
On Mon, Oct 18, 2021 at 02:09:31PM -0700, Tyrel Datwyler wrote: > On 10/14/21 10:03 PM, Gustavo A. R. Silva wrote: > > (!ptr && !ptr->foo) strikes again. :) > > > > The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, > > it leads to a NULL pointer dereference: ptr->foo. > > > > Fix this by converting && to || > > > > This issue was detected with the help of Coccinelle, and audited and > > fixed manually. > > > > Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window operations") > > Cc: stable@vger.kernel.org > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Looking at the usage pattern it is obvious that if we determine !ptr attempting > to also confirm !ptr->ops is going to blow up. > > LGTM. > > Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com> Thanks, Tyrel. -- Gustavo > > > --- > > arch/powerpc/platforms/book3s/vas-api.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c > > index 30172e52e16b..4d82c92ddd52 100644 > > --- a/arch/powerpc/platforms/book3s/vas-api.c > > +++ b/arch/powerpc/platforms/book3s/vas-api.c > > @@ -303,7 +303,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg) > > return -EINVAL; > > } > > > > - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->open_win) { > > + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->open_win) { > > pr_err("VAS API is not registered\n"); > > return -EACCES; > > } > > @@ -373,7 +373,7 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma) > > return -EINVAL; > > } > > > > - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->paste_addr) { > > + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->paste_addr) { > > pr_err("%s(): VAS API is not registered\n", __func__); > > return -EACCES; > > } > > >
On Mon, Oct 18, 2021 at 02:09:31PM -0700, Tyrel Datwyler wrote: > On 10/14/21 10:03 PM, Gustavo A. R. Silva wrote: > > (!ptr && !ptr->foo) strikes again. :) > > > > The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, > > it leads to a NULL pointer dereference: ptr->foo. > > > > Fix this by converting && to || > > > > This issue was detected with the help of Coccinelle, and audited and > > fixed manually. > > > > Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window operations") > > Cc: stable@vger.kernel.org > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Looking at the usage pattern it is obvious that if we determine !ptr attempting > to also confirm !ptr->ops is going to blow up. > > LGTM. > > Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com> I think I'll take this in my tree. Thanks, Tyrel. -- Gustavo
"Gustavo A. R. Silva" <gustavoars@kernel.org> writes: > On Mon, Oct 18, 2021 at 02:09:31PM -0700, Tyrel Datwyler wrote: >> On 10/14/21 10:03 PM, Gustavo A. R. Silva wrote: >> > (!ptr && !ptr->foo) strikes again. :) >> > >> > The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, >> > it leads to a NULL pointer dereference: ptr->foo. >> > >> > Fix this by converting && to || >> > >> > This issue was detected with the help of Coccinelle, and audited and >> > fixed manually. >> > >> > Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window operations") >> > Cc: stable@vger.kernel.org >> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> >> Looking at the usage pattern it is obvious that if we determine !ptr attempting >> to also confirm !ptr->ops is going to blow up. >> >> LGTM. >> >> Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com> > > I think I'll take this in my tree. I've already put it in powerpc/next: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36 If you need to pick it up as well for some reason that's fine. cheers
On Wed, Oct 27, 2021 at 09:30:53AM +1100, Michael Ellerman wrote: [..] > > I think I'll take this in my tree. > > I've already put it in powerpc/next: > > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36 Oh, great. :) > If you need to pick it up as well for some reason that's fine. I just didn't want it to get lost somehow. I'll drop it from tree now. Thanks -- Gustavo
"Gustavo A. R. Silva" <gustavoars@kernel.org> writes: > On Wed, Oct 27, 2021 at 09:30:53AM +1100, Michael Ellerman wrote: > [..] >> > I think I'll take this in my tree. >> >> I've already put it in powerpc/next: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36 > > Oh, great. :) > >> If you need to pick it up as well for some reason that's fine. > > I just didn't want it to get lost somehow. I'll drop it from tree now. No worries, sorry I've been so slow lately. cheers
On Fri, 15 Oct 2021 00:03:45 -0500, Gustavo A. R. Silva wrote: > (!ptr && !ptr->foo) strikes again. :) > > The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, > it leads to a NULL pointer dereference: ptr->foo. > > Fix this by converting && to || > > [...] Applied to powerpc/next. [1/1] powerpc/vas: Fix potential NULL pointer dereference https://git.kernel.org/powerpc/c/61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36 cheers
diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c index 30172e52e16b..4d82c92ddd52 100644 --- a/arch/powerpc/platforms/book3s/vas-api.c +++ b/arch/powerpc/platforms/book3s/vas-api.c @@ -303,7 +303,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg) return -EINVAL; } - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->open_win) { + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->open_win) { pr_err("VAS API is not registered\n"); return -EACCES; } @@ -373,7 +373,7 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma) return -EINVAL; } - if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->paste_addr) { + if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->paste_addr) { pr_err("%s(): VAS API is not registered\n", __func__); return -EACCES; }
(!ptr && !ptr->foo) strikes again. :) The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL, it leads to a NULL pointer dereference: ptr->foo. Fix this by converting && to || This issue was detected with the help of Coccinelle, and audited and fixed manually. Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window operations") Cc: stable@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- arch/powerpc/platforms/book3s/vas-api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)