Hi, Will anyone bother to confirm and fix this problem I reported last time? From the point of view of security, I think it should be fixed. I have discovered several cases of the same kind and all have been fixed by the maintainers. Thanks! Pengfei 在 2016年4月26日，下午5:00，Pengfei Wang <wpengfeinudt@xxxxxxxxx> 写道： Hi, 在 2016年4月26日，下午3:46，James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> 写道： On Tue, 2016-04-26 at 13:35 +0100, Pengfei Wang wrote: Hello, I found this Double-Fetch bug in Linux-4.5/drivers/scsi/aacraid/commctrl.c when I was examining the source code. In function ioctl_send_fib(), the driver fetches user space data by pointer arg via copy_from_user(), and this happens twice at line 81 and line 116 respectively. The first fetched value (stored in kfib) is used to get the header and calculate the size at line 90 so as to copy the whole message later at line 116, which means the copy size of the whole message is based on the old value that came from the first fetch. Besides, the whole message copied in the second fetch also contains the header. However, when the function processes the message after the second fetch at line 130, it uses kfib->header.Size that came from the second fetch, which might be different from the one came from the first fetch as well as calculated the size to copy the message from user space to driver. I don't actually see where there's a bug in this. If the user chooses to alter data in-flight (quite hard to do because one thread of execution is already tied up in the ioctl) then the consequences are their own fault. A Double-Fetch happens when the kernel reads the same user data multiple times, whilst the data is likely to be modified by a concurrently running user thread under race condition between the kernel reads, which results in data inconsistency for the kernel use. Since neither the kernel nor the user is aware of the change of the data under race condition, this data inconsistency might cause serious consequences. I call this Double-Fetch situation as a bug is because I think it lacks a sanity checking after the second fetch of the same data, which should be a guarantee for the data consistency for kernel use, and I have seen this checking mechanism in some other driver files. Moreover, I don’t think it is the user’s fault if the data is altered, for the reason that the data might be altered by an intended malicious process launched by an attacker or rewritten by some unintended user behaviors that are not supposed to happen. As long as the data resides in user space, there is a chance that it is modified under race condition, even though the chance might be very small. Besides, it should be the kernel’s duty to handle potential data inconsistency in Double-Fetch. If the kfib->header.Size 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 over-boundary access or other serious consequences in function aac_fib_send(). Our only real concern would be could an unprivileged user crash the kernel this way and the user must have CAP_SYS_RAWIO anyway (which is quite a high privilege capability) plus the only problem with shortening the data would be EFAULT. I’m not quite sure what consequence will actually be caused if this Double-Fetch really happens, maybe not as serious as a kernel crash, but I think it would actually results in something wrong in the driver functionality, which should be eliminated now. Thank you Pengfei -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
