Re: [PATCH 1/2] staging: vchiq: fix __user annotations

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Sep 22, 2020 at 10:21:43PM +0200, Arnd Bergmann wrote:
> My earlier patches caused some new sparse warnings, but it turns out
> that a number of those are actual bugs, or at least suspicous code.
> 
> Adding __user annotations to the data structures that are defined in
> uapi headers helps avoid the new warnings, but that causes a different
> set of warnings to show up, as some of these structures are used both
> inside of the kernel and at the user interface but storing pointers to
> different things there.
> 
> Duplicating the vchiq_service_params and vchiq_completion_data structures
> in turn takes care of most of those, and then it turns out that there
> is a 'data' pointer that can be any of a __user address, a dmd_addr_t
> and a kernel pointer in vmalloc space at times.
> 
> I'm trying to annotate these as best I can without changing behavior,
> but there still seems to be a serious bug when user space passes
> a valid vmalloc space address instead of a user pointer. Adding
> comments in the code there, and leaving the warnings in place that
> seem to correspond to actual bugs.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  .../include/linux/raspberrypi/vchiq.h         | 11 ++-
>  .../interface/vchiq_arm/vchiq_2835_arm.c      |  2 +-
>  .../interface/vchiq_arm/vchiq_arm.c           | 95 ++++++++++++-------
>  .../interface/vchiq_arm/vchiq_core.c          | 19 ++--
>  .../interface/vchiq_arm/vchiq_core.h          | 10 +-
>  .../interface/vchiq_arm/vchiq_ioctl.h         | 29 ++++--
>  6 files changed, 106 insertions(+), 60 deletions(-)

This patch series breaks the build for me:

drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c: In function ‘vc_vchi_audio_init’:
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c:125:9: error: variable ‘param
’ has initializer but incomplete type
  125 |  struct vchiq_service_params params = {
      |         ^~~~~~~~~~~~~~~~~~~~
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c:126:4: error: ‘struct vchiq_service_params’ has no member named ‘version’
  126 |   .version  = VC_AUDIOSERV_VER,
      |    ^~~~~~~
In file included from drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c:8:
drivers/staging/vc04_services/bcm2835-audio/vc_vchi_audioserv_defs.h:8:26: warning: excess elements in struct initializer
    8 | #define VC_AUDIOSERV_VER 2
      |                          ^
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c:126:15: note: in expansion of macro ‘VC_AUDIOSERV_VER’
  126 |   .version  = VC_AUDIOSERV_VER,
      |               ^~~~~~~~~~~~~~~~


and so on...

Care to try a v2?

thanks,

greg k-h