Bug 116651

Summary: Double-Fetch bug in Linux-4.5/drivers/misc/mic/host/mic_virtio.c
Product: Drivers Reporter: Pengfei Wang (wpengfeinudt)
Component: OtherAssignee: drivers_other
Status: NEW ---    
Severity: high CC: wpengfeinudt
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 4.5 Subsystem:
Regression: No Bisected commit-id:
Attachments: source file

Description Pengfei Wang 2016-04-18 13:30:19 UTC
Hi,

I found this Double-Fetch bug in Linux-4.5/drivers/misc/mic/host/mic_virtio.c when I was examining the source code. 

In function mic_copy_dp_entry(), the driver fetches user space data by pointer argp via copy_from_user(), and this happens twice at line 533 and line 552 respectively. The first fetched value (stored in dd) is used to calculate and allocate buffer(i.e. dd_config) at line 546, which means the buffer is based on the old header dd that came from the first fetch. The second fetch copies again the whole message based on the calculated size, which also includes the header.

However, when the function processes the whole message after the second fetch, it uses the size calculated from the new header in the second fetch, which might be different from the one calculated from the old header and allocated the buffer. This happens both at line 571 and line 591 (both function mic_desc_size(dd_config) and function mic_total_desc_size(dd_config) involve with using the size element num_vq in the new header, and we can see it from file mic_virtio.h).

If the header is modified by a user thread under race condition between the fetch operations, for example changing to a very large value, this will lead to consequence like over-boundary access on the buffer, information leakage or crash (caused by memcpy() at line 591).

I am looking forward to a reply on this, thank you!

Kind regards
Pengfei
Comment 1 Pengfei Wang 2016-04-25 16:23:47 UTC
Created attachment 214121 [details]
source file