[PATCH] Fix stack memory disclosure

Using [1] for static analysis I found that the OMAPFB_QUERY_PLANE,
OMAPFB_GET_COLOR_KEY, OMAPFB_GET_DISPLAY_INFO, and OMAPFB_GET_VRAM_INFO
cases could all leak uninitialized stack memory--either due to
uninitialized padding or 'reserved' fields.

Fix them by clearing the shared union used to store copied out data.

[1] https://github.com/vlad902/kernel-uninitialized-memory-checker

Signed-off-by: Vlad Tsyrklevich <redacted>
Cc: security@kernel.org

---
 drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
index 53f93616c..8e23160ec 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c

@@ -609,6 +609,8 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 
 	int r = 0;
 
+	memset(&p, 0, sizeof(p));
+
 	switch (cmd) {
 	case OMAPFB_SYNC_GFX:
 		DBG("ioctl SYNC_GFX\n");

-- 
2.17.0

On Sat, Jan 5, 2019 at 11:51 PM Vlad Tsyrklevich [off-list ref] wrote:

Using [1] for static analysis I found that the OMAPFB_QUERY_PLANE,
OMAPFB_GET_COLOR_KEY, OMAPFB_GET_DISPLAY_INFO, and OMAPFB_GET_VRAM_INFO
cases could all leak uninitialized stack memory--either due to
uninitialized padding or 'reserved' fields.

Fix them by clearing the shared union used to store copied out data.

[1] https://github.com/vlad902/kernel-uninitialized-memory-checker

Signed-off-by: Vlad Tsyrklevich <redacted>

Reviewed-by: Kees Cook <redacted>
Fixes: b39a982ddecf ("OMAP: DSS2: omapfb driver")

Since this driver is orphaned (according to MAINTAINERS), I think this
fix should go via Bart's tree. Bart, can you take this?

-Kees

-- 
Kees Cook

On 01/09/2019 12:41 AM, Kees Cook wrote:

On Sat, Jan 5, 2019 at 11:51 PM Vlad Tsyrklevich [off-list ref] wrote:

quoted

Using [1] for static analysis I found that the OMAPFB_QUERY_PLANE,
OMAPFB_GET_COLOR_KEY, OMAPFB_GET_DISPLAY_INFO, and OMAPFB_GET_VRAM_INFO
cases could all leak uninitialized stack memory--either due to
uninitialized padding or 'reserved' fields.

Fix them by clearing the shared union used to store copied out data.

[1] https://github.com/vlad902/kernel-uninitialized-memory-checker

Signed-off-by: Vlad Tsyrklevich <redacted>

Reviewed-by: Kees Cook <redacted>
Fixes: b39a982ddecf ("OMAP: DSS2: omapfb driver")

Thanks.

Since this driver is orphaned (according to MAINTAINERS), I think this
fix should go via Bart's tree. Bart, can you take this?

Sure, I will merge it tomorrow (I plan to send fbdev fixes pull request to
Linus next week).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

-Kees

quoted

Cc: security@kernel.org

---
 drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
index 53f93616c..8e23160ec 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c

@@ -609,6 +609,8 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)

        int r = 0;

+       memset(&p, 0, sizeof(p));
+
        switch (cmd) {
        case OMAPFB_SYNC_GFX:
                DBG("ioctl SYNC_GFX\n");
--

2.17.0

On 01/10/2019 04:10 PM, Bartlomiej Zolnierkiewicz wrote:

On 01/09/2019 12:41 AM, Kees Cook wrote:

quoted

On Sat, Jan 5, 2019 at 11:51 PM Vlad Tsyrklevich [off-list ref] wrote:

quoted

Using [1] for static analysis I found that the OMAPFB_QUERY_PLANE,
OMAPFB_GET_COLOR_KEY, OMAPFB_GET_DISPLAY_INFO, and OMAPFB_GET_VRAM_INFO
cases could all leak uninitialized stack memory--either due to
uninitialized padding or 'reserved' fields.

Fix them by clearing the shared union used to store copied out data.

[1] https://github.com/vlad902/kernel-uninitialized-memory-checker

Signed-off-by: Vlad Tsyrklevich <redacted>

Reviewed-by: Kees Cook <redacted>
Fixes: b39a982ddecf ("OMAP: DSS2: omapfb driver")

Thanks.

quoted

Since this driver is orphaned (according to MAINTAINERS), I think this
fix should go via Bart's tree. Bart, can you take this?

Sure, I will merge it tomorrow (I plan to send fbdev fixes pull request to
Linus next week).

Patch queued for 5.0 (with patch subject prefixed with "omap2fb: "), thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics