* [patch] /drivers/iommu/amd_iommu.c: variable unmap_size could be uninitialized
@ 2018-05-31 6:02 Yizhuo Zhai
2018-05-31 9:44 ` Joerg Roedel
0 siblings, 1 reply; 3+ messages in thread
From: Yizhuo Zhai @ 2018-05-31 6:02 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Chengyu Song, Zhiyun Qian
[-- Attachment #1.1: Type: text/plain, Size: 992 bytes --]
Variable "unmap_size" is supposed to be initialized in function fetch_pte.
However, it's uninitialized if fetch_pte returns NULL. And "unmap_size" is
used outside the return check.
>From 377ccb647d3c6c6747f20a242b035bafc775c3be Mon Sep 17 00:00:00 2001
Signed-off-by: From: "yzhai003-3vSkeFsW7jA@public.gmane.org" <yzhai003-3vSkeFsW7jA@public.gmane.org>
---
drivers/iommu/amd_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8fb8c73..774e057 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1500,7 +1500,7 @@ static unsigned long iommu_unmap_page(struct
protection_domain *dom,
unsigned long page_size)
{
unsigned long long unmapped;
- unsigned long unmap_size;
+ unsigned long unmap_size = 0;
u64 *pte;
BUG_ON(!is_power_of_2(page_size));
--
2.7.4
--
Kind Regards,
*Yizhuo Zhai*
*Computer Science, Graduate Student*
*University of California, Riverside *
[-- Attachment #1.2: Type: text/html, Size: 2225 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [patch] /drivers/iommu/amd_iommu.c: variable unmap_size could be uninitialized
2018-05-31 6:02 [patch] /drivers/iommu/amd_iommu.c: variable unmap_size could be uninitialized Yizhuo Zhai
@ 2018-05-31 9:44 ` Joerg Roedel
[not found] ` <20180531094415.GR18595-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Joerg Roedel @ 2018-05-31 9:44 UTC (permalink / raw)
To: Yizhuo Zhai; +Cc: iommu, linux-kernel, Chengyu Song, Zhiyun Qian
Hi Yizhuo Zhai,
thanks for your patch, but I think there is a better way to fix that.
Please see below.
On Wed, May 30, 2018 at 11:02:54PM -0700, Yizhuo Zhai wrote:
> Variable "unmap_size" is supposed to be initialized in function fetch_pte.
> However, it's uninitialized if fetch_pte returns NULL. And "unmap_size" is used
> outside the return check.
>
> From 377ccb647d3c6c6747f20a242b035bafc775c3be Mon Sep 17 00:00:00 2001
>
> Signed-off-by: From: "yzhai003@ucr.edu" <yzhai003@ucr.edu>
> ---
> drivers/iommu/amd_iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 8fb8c73..774e057 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -1500,7 +1500,7 @@ static unsigned long iommu_unmap_page(struct
> protection_domain *dom,
> unsigned long page_size)
> {
> unsigned long long unmapped;
> - unsigned long unmap_size;
> + unsigned long unmap_size = 0;
That is not sufficient because fetch_pte is called in a loop, and when
it returns NULL then unmap_size keeps the value of the previous
invocation.
So from looking at the code, it is better to set page_size=0 in
fetch_pte at the very beginning, before the function can return NULL.
Thanks,
Joerg
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] /drivers/iommu/amd_iommu.c: variable unmap_size could be uninitialized
[not found] ` <20180531094415.GR18595-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
@ 2018-05-31 18:21 ` Yizhuo Zhai
0 siblings, 0 replies; 3+ messages in thread
From: Yizhuo Zhai @ 2018-05-31 18:21 UTC (permalink / raw)
To: Joerg Roedel
Cc: Chengyu Song, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Zhiyun Qian, linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1.1: Type: text/plain, Size: 1689 bytes --]
Yes, thank you for your advice. The new patch's been sent.
On Thu, May 31, 2018 at 2:44 AM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
> Hi Yizhuo Zhai,
>
> thanks for your patch, but I think there is a better way to fix that.
> Please see below.
>
> On Wed, May 30, 2018 at 11:02:54PM -0700, Yizhuo Zhai wrote:
> > Variable "unmap_size" is supposed to be initialized in function
> fetch_pte.
> > However, it's uninitialized if fetch_pte returns NULL. And "unmap_size"
> is used
> > outside the return check.
> >
> > From 377ccb647d3c6c6747f20a242b035bafc775c3be Mon Sep 17 00:00:00 2001
> >
> > Signed-off-by: From: "yzhai003-3vSkeFsW7jA@public.gmane.org" <yzhai003-3vSkeFsW7jA@public.gmane.org>
> > ---
> > drivers/iommu/amd_iommu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> > index 8fb8c73..774e057 100644
> > --- a/drivers/iommu/amd_iommu.c
> > +++ b/drivers/iommu/amd_iommu.c
> > @@ -1500,7 +1500,7 @@ static unsigned long iommu_unmap_page(struct
> > protection_domain *dom,
> > unsigned long page_size)
> > {
> > unsigned long long unmapped;
> > - unsigned long unmap_size;
> > + unsigned long unmap_size = 0;
>
> That is not sufficient because fetch_pte is called in a loop, and when
> it returns NULL then unmap_size keeps the value of the previous
> invocation.
>
> So from looking at the code, it is better to set page_size=0 in
> fetch_pte at the very beginning, before the function can return NULL.
>
>
> Thanks,
>
> Joerg
>
>
--
Kind Regards,
*Yizhuo Zhai*
*Computer Science, Graduate Student*
*University of California, Riverside *
[-- Attachment #1.2: Type: text/html, Size: 2767 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-31 18:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31 6:02 [patch] /drivers/iommu/amd_iommu.c: variable unmap_size could be uninitialized Yizhuo Zhai
2018-05-31 9:44 ` Joerg Roedel
[not found] ` <20180531094415.GR18595-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2018-05-31 18:21 ` Yizhuo Zhai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.