[PATCH] media: davinci_vpfe: fix a NULL pointer dereference bug
In vpfe_isif_init(), there is a while loop to get the ISIF base address and
linearization table0 and table1 address. In the loop body, the function
platform_get_resource() is called to get the resource. If
platform_get_resource() returns NULL, the loop is terminated and the
execution goes to 'fail_nobase_res'. Suppose the loop is terminated at the
first iteration because platform_get_resource() returns NULL and the
execution goes to 'fail_nobase_res'. Given that there is another while loop
at 'fail_nobase_res' and i equals to 0, one iteration of the second while
loop will be executed. However, the second while loop does not check the
return value of platform_get_resource(). This can cause a NULL pointer
dereference bug if the return value is a NULL pointer.
This patch avoids the above issue by adding a check in the second while
loop after the call to platform_get_resource().
Signed-off-by: Wenwen Wang
---
drivers/staging/media/davinci_vpfe/dm365_isif.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
b/drivers/staging/media/davinci_vpfe/dm365_isif.c
index 745e33f..b0425a6 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
@@ -2080,7 +2080,8 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct
platform_device *pdev)
while (i >= 0) {
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
- release_mem_region(res->start, res_len);
+ if (res)
+ release_mem_region(res->start, res_len);
i--;
}
return status;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: davinci_vpfe: fix a NULL pointer dereference bug
Hello,
Can anyone please confirm this bug and apply the patch? Thanks!
Wenwen
On Thu, Oct 4, 2018 at 11:00 AM Wenwen Wang wrote:
>
> In vpfe_isif_init(), there is a while loop to get the ISIF base address and
> linearization table0 and table1 address. In the loop body, the function
> platform_get_resource() is called to get the resource. If
> platform_get_resource() returns NULL, the loop is terminated and the
> execution goes to 'fail_nobase_res'. Suppose the loop is terminated at the
> first iteration because platform_get_resource() returns NULL and the
> execution goes to 'fail_nobase_res'. Given that there is another while loop
> at 'fail_nobase_res' and i equals to 0, one iteration of the second while
> loop will be executed. However, the second while loop does not check the
> return value of platform_get_resource(). This can cause a NULL pointer
> dereference bug if the return value is a NULL pointer.
>
> This patch avoids the above issue by adding a check in the second while
> loop after the call to platform_get_resource().
>
> Signed-off-by: Wenwen Wang
> ---
> drivers/staging/media/davinci_vpfe/dm365_isif.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> index 745e33f..b0425a6 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> @@ -2080,7 +2080,8 @@ int vpfe_isif_init(struct vpfe_isif_device *isif,
> struct platform_device *pdev)
>
> while (i >= 0) {
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> - release_mem_region(res->start, res_len);
> + if (res)
> + release_mem_region(res->start, res_len);
> i--;
> }
> return status;
> --
> 2.7.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: luster: llite: fix a potential missing-check bug when copying lumv
In ll_dir_ioctl(), the object lumv3 is firstly copied from the user space
using Its address, i.e., lumv1 = &lumv3. If the lmm_magic field of lumv3 is
LOV_USER_MAGIV_V3, lumv3 will be modified by the second copy from the user
space. The second copy is necessary, because the two versions (i.e.,
lov_user_md_v1 and lov_user_md_v3) have different data formats and lengths.
However, given that the user data resides in the user space, a malicious
user-space process can race to change the data between the two copies. By
doing so, the attacker can provide a data with an inconsistent version,
e.g., v1 version + v3 data. This can lead to logical errors in the
following execution in ll_dir_setstripe(), which performs different actions
according to the version specified by the field lmm_magic.
This patch rechecks the version field lmm_magic in the second copy. If the
version is not as expected, i.e., LOV_USER_MAGIC_V3, an error code will be
returned: -EINVAL.
Signed-off-by: Wenwen Wang
---
drivers/staging/lustre/lustre/llite/dir.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/lustre/lustre/llite/dir.c
b/drivers/staging/lustre/lustre/llite/dir.c
index d10d272..80d44ca 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -1185,6 +1185,8 @@ static long ll_dir_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (lumv1->lmm_magic == LOV_USER_MAGIC_V3) {
if (copy_from_user(&lumv3, lumv3p, sizeof(lumv3)))
return -EFAULT;
+ if (lumv3.lmm_magic != LOV_USER_MAGIC_V3)
+ return -EINVAL;
}
if (is_root_inode(inode))
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: atomisp: fix a potential missing-check bug
At the end of atomisp_subdev_set_selection(), the function atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since this function may return a NULL pointer, it is firstly invoked to check the returned pointer. If the returned pointer is not NULL, then the function is invoked again to obtain the pointer and the memory content at the location of the returned pointer is copied to the memory location of r. In most cases, the pointers returned by the two invocations are same. However, given that the pointer returned by the function atomisp_subdev_get_rect() is not a constant, it is possible that the two invocations return two different pointers. For example, another thread may race to modify the related pointers during the two invocations. In that case, even if the first returned pointer is not null, the second returned pointer might be null, which will cause issues such as null pointer dereference. This patch saves the pointer returned by the first invocation and removes the second invocation. If the returned pointer is not NULL, the memory content is copied according to the original code. Signed-off-by: Wenwen Wang --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c index 49a9973..d5fa513 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c @@ -366,6 +366,7 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, unsigned int i; unsigned int padding_w = pad_w; unsigned int padding_h = pad_h; + struct v4l2_rect *p; stream_id = atomisp_source_pad_to_stream_id(isp_sd, vdev_pad); @@ -536,9 +537,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, ffmt[pad]->height = comp[pad]->height; } - if (!atomisp_subdev_get_rect(sd, cfg, which, pad, target)) + p = atomisp_subdev_get_rect(sd, cfg, which, pad, target); + if (!p) return -EINVAL; - *r = *atomisp_subdev_get_rect(sd, cfg, which, pad, target); + *r = *p; dev_dbg(isp->dev, "sel actual: l %d t %d w %d h %d\n", r->left, r->top, r->width, r->height); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: luster: llite: fix a potential missing-check bug when copying lumv
On Sun, Apr 29, 2018 at 8:20 AM, Greg Kroah-Hartman wrote: > On Sat, Apr 28, 2018 at 04:04:25PM +, Dilger, Andreas wrote: >> On Apr 27, 2018, at 17:45, Wenwen Wang wrote: >> > [PATCH] staging: luster: llite: fix potential missing-check bug when >> > copying lumv >> >> (typo) s/luster/lustre/ >> >> > In ll_dir_ioctl(), the object lumv3 is firstly copied from the user space >> > using Its address, i.e., lumv1 = &lumv3. If the lmm_magic field of lumv3 is >> > LOV_USER_MAGIV_V3, lumv3 will be modified by the second copy from the user >> >> (typo) s/MAGIV/MAGIC/ >> >> > space. The second copy is necessary, because the two versions (i.e., >> > lov_user_md_v1 and lov_user_md_v3) have different data formats and lengths. >> > However, given that the user data resides in the user space, a malicious >> > user-space process can race to change the data between the two copies. By >> > doing so, the attacker can provide a data with an inconsistent version, >> > e.g., v1 version + v3 data. This can lead to logical errors in the >> > following execution in ll_dir_setstripe(), which performs different actions >> > according to the version specified by the field lmm_magic. >> >> This isn't a serious bug in the end. The LOV_USER_MAGIC_V3 check just copies >> a bit more data from userspace (the lmm_pool field). It would be more of a >> problem if the reverse was possible (copy smaller V1 buffer, but change the >> magic to LOV_USER_MAGIC_V3 afterward), but this isn't possible since the >> second >> copy is not done if there is a V1 magic. If the user changes from V3 magic >> to V1 in a racy manner it means less data will be used than copied, which >> is harmless. >> >> > This patch rechecks the version field lmm_magic in the second copy. If the >> > version is not as expected, i.e., LOV_USER_MAGIC_V3, an error code will be >> > returned: -EINVAL. >> >> This isn't a bad idea in any case, since it verifies the data copied from >> userspace is still valid. > > So you agree with this patch? Or do not? > > confused, > > greg k-h It is worth fixing this bug, since it offers an opportunity for adversaries to provide inconsistent user data. In addition to the unwanted version LOV_USER_MAGIC_V1, a malicious user can also use the version LMV_USER_MAGIC, which is also unexpected but allowed in the function ll_dir_setstripe(). These inconsistent data can cause potential logical errors in the following execution. Hence it is necessary to re-verify the data copied from userspace. Thanks! Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: luster: llite: fix a potential missing-check bug when copying lumv
On Mon, Apr 30, 2018 at 5:38 PM, Dilger, Andreas wrote: > On Apr 29, 2018, at 07:20, Greg Kroah-Hartman > wrote: >> >> On Sat, Apr 28, 2018 at 04:04:25PM +, Dilger, Andreas wrote: >>> On Apr 27, 2018, at 17:45, Wenwen Wang wrote: >>>> [PATCH] staging: luster: llite: fix potential missing-check bug when >>>> copying lumv >>> >>> (typo) s/luster/lustre/ >>> >>>> In ll_dir_ioctl(), the object lumv3 is firstly copied from the user space >>>> using Its address, i.e., lumv1 = &lumv3. If the lmm_magic field of lumv3 is >>>> LOV_USER_MAGIV_V3, lumv3 will be modified by the second copy from the user >>> >>> (typo) s/MAGIV/MAGIC/ >>> >>>> space. The second copy is necessary, because the two versions (i.e., >>>> lov_user_md_v1 and lov_user_md_v3) have different data formats and lengths. >>>> However, given that the user data resides in the user space, a malicious >>>> user-space process can race to change the data between the two copies. By >>>> doing so, the attacker can provide a data with an inconsistent version, >>>> e.g., v1 version + v3 data. This can lead to logical errors in the >>>> following execution in ll_dir_setstripe(), which performs different actions >>>> according to the version specified by the field lmm_magic. >>> >>> This isn't a serious bug in the end. The LOV_USER_MAGIC_V3 check just >>> copies >>> a bit more data from userspace (the lmm_pool field). It would be more of a >>> problem if the reverse was possible (copy smaller V1 buffer, but change the >>> magic to LOV_USER_MAGIC_V3 afterward), but this isn't possible since the >>> second >>> copy is not done if there is a V1 magic. If the user changes from V3 magic >>> to V1 in a racy manner it means less data will be used than copied, which >>> is harmless. >>> >>>> This patch rechecks the version field lmm_magic in the second copy. If the >>>> version is not as expected, i.e., LOV_USER_MAGIC_V3, an error code will be >>>> returned: -EINVAL. >>> >>> This isn't a bad idea in any case, since it verifies the data copied from >>> userspace is still valid. >> >> So you agree with this patch? Or do not? >> >> confused, > > I don't think it fixes a real bug, but it makes the code a bit more clear, > so I'm OK to land it (with minor corrections to commit message per above). > > Cheers, Andreas > -- > Andreas Dilger > Lustre Principal Architect > Intel Corporation > Thanks! I will re-submit the patch with the corrected commit message. Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: lustre: llite: fix potential missing-check bug when copying lumv
In ll_dir_ioctl(), the object lumv3 is firstly copied from the user space
using Its address, i.e., lumv1 = &lumv3. If the lmm_magic field of lumv3 is
LOV_USER_MAGIC_V3, lumv3 will be modified by the second copy from the user
space. The second copy is necessary, because the two versions (i.e.,
lov_user_md_v1 and lov_user_md_v3) have different data formats and lengths.
However, given that the user data resides in the user space, a malicious
user-space process can race to change the data between the two copies. By
doing so, the attacker can provide a data with an inconsistent version,
e.g., v1 version + v3 data. This can lead to logical errors in the
following execution in ll_dir_setstripe(), which performs different actions
according to the version specified by the field lmm_magic.
This patch rechecks the version field lmm_magic in the second copy. If the
version is not as expected, i.e., LOV_USER_MAGIC_V3, an error code will be
returned: -EINVAL.
Signed-off-by: Wenwen Wang
---
drivers/staging/lustre/lustre/llite/dir.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/lustre/lustre/llite/dir.c
b/drivers/staging/lustre/lustre/llite/dir.c
index d10d272..80d44ca 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -1185,6 +1185,8 @@ static long ll_dir_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (lumv1->lmm_magic == LOV_USER_MAGIC_V3) {
if (copy_from_user(&lumv3, lumv3p, sizeof(lumv3)))
return -EFAULT;
+ if (lumv3.lmm_magic != LOV_USER_MAGIC_V3)
+ return -EINVAL;
}
if (is_root_inode(inode))
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: atomisp: fix a potential missing-check bug
At the end of atomisp_subdev_set_selection(), the function atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since this function may return a NULL pointer, it is firstly invoked to check the returned pointer. If the returned pointer is not NULL, then the function is invoked again to obtain the pointer and the memory content at the location of the returned pointer is copied to the memory location of r. In most cases, the pointers returned by the two invocations are same. However, given that the pointer returned by the function atomisp_subdev_get_rect() is not a constant, it is possible that the two invocations return two different pointers. For example, another thread may race to modify the related pointers during the two invocations. In that case, even if the first returned pointer is not null, the second returned pointer might be null, which will cause issues such as null pointer dereference. This patch saves the pointer returned by the first invocation and removes the second invocation. If the returned pointer is not NULL, the memory content is copied according to the original code. Signed-off-by: Wenwen Wang --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c index 49a9973..d5fa513 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c @@ -366,6 +366,7 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, unsigned int i; unsigned int padding_w = pad_w; unsigned int padding_h = pad_h; + struct v4l2_rect *p; stream_id = atomisp_source_pad_to_stream_id(isp_sd, vdev_pad); @@ -536,9 +537,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, ffmt[pad]->height = comp[pad]->height; } - if (!atomisp_subdev_get_rect(sd, cfg, which, pad, target)) + p = atomisp_subdev_get_rect(sd, cfg, which, pad, target); + if (!p) return -EINVAL; - *r = *atomisp_subdev_get_rect(sd, cfg, which, pad, target); + *r = *p; dev_dbg(isp->dev, "sel actual: l %d t %d w %d h %d\n", r->left, r->top, r->width, r->height); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: llite: fix potential missing-check bug when copying lumv
On Tue, May 1, 2018 at 3:46 AM, Dan Carpenter wrote: > On Mon, Apr 30, 2018 at 05:56:10PM -0500, Wenwen Wang wrote: >> However, given that the user data resides in the user space, a malicious >> user-space process can race to change the data between the two copies. By >> doing so, the attacker can provide a data with an inconsistent version, >> e.g., v1 version + v3 data. This can lead to logical errors in the >> following execution in ll_dir_setstripe(), which performs different actions >> according to the version specified by the field lmm_magic. > > This part is misleading. The fix is to improve readability and make > static checkers happy. You're over dramatizing it to make people think > it has a security impact when it doesn't. > > If the user wants to specify v1 data they can just say that on the first > read. They don't need to do funny tricks and race between the two > reads. It's allowed. > > In other words this allows the user to do something in a very > complicated way which they are already allowed to do in a very simple > straight forward way. > > regards, > dan carpenter Thanks for your comment, Dan! How about this: However, given that the user data resides in the user space, a malicious user-space process can race to change the data between the two copies. By doing so, the attacker can provide a data with an inconsistent version, e.g., v1 version + v3 data. The current kernel can handle such inconsistent data. But, it may pose a potential security risk for future implementations. Also, to improve code readability and make static analysis tools happy, which will warn about read-verify-re-read type bugs, this issue should be fixed. Thanks, Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: llite: fix potential missing-check bug when copying lumv
On Fri, May 4, 2018 at 12:27 AM, Dan Carpenter wrote: > There is no security problem here. The user is allowed to choose either > v1 or v3. Using a double read race condition to choose v1 is not > going to cause problems. It's slightly more complicated than just > choosing it directly but that doesn't make it a security issue. > > It's a bit like typing with your feet in that just because using your > toes instead of your fingergs is more complicated, it doesn't make it a > security issue. > > regards, > dan carpenter > Thanks again for your comment, Dan! I revised the commit message and removed the security risk: However, given that the user data resides in the user space, a malicious user-space process can race to change the data between the two copies. By doing so, the user can provide a data with an inconsistent version, e.g., v1 version + v3 data. To improve code readability and make static analysis tools happy, which will warn about read-verify-re-read type bugs, this issue should be fixed. Thanks, Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging: atomisp: fix a potential missing-check bug
At the end of atomisp_subdev_set_selection(), the function atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since this function may return a NULL pointer, it is firstly invoked to check the returned pointer. If the returned pointer is not NULL, then the function is invoked again to obtain the pointer and the memory content at the location of the returned pointer is copied to the memory location of r. In most cases, the pointers returned by the two invocations are same. However, given that the pointer returned by the function atomisp_subdev_get_rect() is not a constant, it is possible that the two invocations return two different pointers. For example, another thread may race to modify the related pointers during the two invocations. In that case, even if the first returned pointer is not null, the second returned pointer might be null, which will cause issues such as null pointer dereference. This patch saves the pointer returned by the first invocation and removes the second invocation. If the returned pointer is not NULL, the memory content is copied according to the original code. Signed-off-by: Wenwen Wang --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c index 49a9973..d5fa513 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c @@ -366,6 +366,7 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, unsigned int i; unsigned int padding_w = pad_w; unsigned int padding_h = pad_h; + struct v4l2_rect *p; stream_id = atomisp_source_pad_to_stream_id(isp_sd, vdev_pad); @@ -536,9 +537,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd, ffmt[pad]->height = comp[pad]->height; } - if (!atomisp_subdev_get_rect(sd, cfg, which, pad, target)) + p = atomisp_subdev_get_rect(sd, cfg, which, pad, target); + if (!p) return -EINVAL; - *r = *atomisp_subdev_get_rect(sd, cfg, which, pad, target); + *r = *p; dev_dbg(isp->dev, "sel actual: l %d t %d w %d h %d\n", r->left, r->top, r->width, r->height); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: llite: fix potential missing-check bug when copying lumv
On Fri, May 4, 2018 at 5:08 AM, Dilger, Andreas wrote: > On May 3, 2018, at 22:19, Wenwen Wang wrote: >> >> On Tue, May 1, 2018 at 3:46 AM, Dan Carpenter >> wrote: >>> On Mon, Apr 30, 2018 at 05:56:10PM -0500, Wenwen Wang wrote: >>>> However, given that the user data resides in the user space, a malicious >>>> user-space process can race to change the data between the two copies. By >>>> doing so, the attacker can provide a data with an inconsistent version, >>>> e.g., v1 version + v3 data. This can lead to logical errors in the >>>> following execution in ll_dir_setstripe(), which performs different actions >>>> according to the version specified by the field lmm_magic. >>> >>> This part is misleading. The fix is to improve readability and make >>> static checkers happy. You're over dramatizing it to make people think >>> it has a security impact when it doesn't. >>> >>> If the user wants to specify v1 data they can just say that on the first >>> read. They don't need to do funny tricks and race between the two >>> reads. It's allowed. >>> >>> In other words this allows the user to do something in a very >>> complicated way which they are already allowed to do in a very simple >>> straight forward way. >>> >>> regards, >>> dan carpenter >> >> Thanks for your comment, Dan! How about this: >> >> However, given that the user data resides in the user space, a >> malicious user-space process can race to change the data between the >> two copies. By doing so, the attacker can provide a data with an >> inconsistent version, e.g., v1 version + v3 data. The current kernel >> can handle such inconsistent data. But, it may pose a potential >> security risk for future implementations. Also, to improve code >> readability and make static analysis tools happy, which will warn >> about read-verify-re-read type bugs, this issue should be fixed. > > There is nothing preventing the user from using struct lov_mds_md_v3 but > filling in lmm_magic = LOV_MAGIC_V1 from the beginning, no need for a race. > But, if the user uses such a struct, the second fetch won't be executed. This is a little bit different from using LOV_MAGIC_V3 firstly and then changing it to LOV_MAGIC_V1. Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: staging: atomisp: fix a potential missing-check bug
On Tue, May 8, 2018 at 7:16 AM, Dan Carpenter wrote: > On Wed, May 02, 2018 at 05:38:49PM -0500, Wenwen Wang wrote: >> At the end of atomisp_subdev_set_selection(), the function >> atomisp_subdev_get_rect() is invoked to get the pointer to v4l2_rect. Since >> this function may return a NULL pointer, it is firstly invoked to check >> the returned pointer. If the returned pointer is not NULL, then the >> function is invoked again to obtain the pointer and the memory content >> at the location of the returned pointer is copied to the memory location of >> r. In most cases, the pointers returned by the two invocations are same. >> However, given that the pointer returned by the function >> atomisp_subdev_get_rect() is not a constant, it is possible that the two >> invocations return two different pointers. For example, another thread may >> race to modify the related pointers during the two invocations. > > You're assuming a very serious race condition exists. > > >> In that >> case, even if the first returned pointer is not null, the second returned >> pointer might be null, which will cause issues such as null pointer >> dereference. > > And then complaining that if a really serious bug exists then this very > minor bug would exist too... If there were really a race condition like > that then we'd want to fix it instead. In other words, this is not a > real life bug fix. > > But it would be fine as a readability or static checker fix so that's > fine. Thanks for your response. From the performance perspective, this bug should also be fixed, as the second invocation is redundant if it is expected to return a same pointer as the first one. Wenwen ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
