Hi

Am 03.05.23 um 17:02 schrieb Geert Uytterhoeven:

Hi Thomas,

On Wed, May 3, 2023 at 4:30 PM Thomas Zimmermann [off-list ref] wrote:

quoted

Am 03.05.23 um 11:51 schrieb Geert Uytterhoeven:

quoted

On Fri, Apr 28, 2023 at 2:26 PM Thomas Zimmermann [off-list ref] wrote:

quoted

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 the

both

quoted

same 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)