Hi Am 03.05.23 um 17:02 schrieb Geert Uytterhoeven:
Hi Thomas, On Wed, May 3, 2023 at 4:30 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:Am 03.05.23 um 11:51 schrieb Geert Uytterhoeven:On Fri, Apr 28, 2023 at 2:26 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:Push the test for info->screen_base from fb_read() and fb_write() into the implementations of struct fb_ops.{fb_read,fb_write}. In cases where the driver operates on info->screen_buffer, test this field instead. While bothi fields, screen_base and screen_buffer, are stored in thebothsame location, they refer to different address spaces. For correctness, we want to test each field in exactly the code that uses it.Not a direct comment for this patch: and later the union can be split in two separate fields, to protect against misuse?No idea. Currently we have sparse that warns about mismatching address spaces if the fields are mixed up. That's good enough, as far I'm concerned.The potential issue that is still present is that an fbdev driver uses fb_info.screen_base, and configures the use of drawing ops that use fb_info.screen_buffer (or vice-versa), which will happily use the wrong type of pointer. Sparse doesn't protect against that.
Right. From a quick grep, I've found quite a cases where cfb_ functions operate on non-__iomem memory. I'm sure that the opposite with sys_ functions exists as well. Fixing this will be a good follow-up patchset. Thanks for the suggestion.
Best regards Thomas
Gr{oetje,eeting}s,
Geert
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
