vchiq: Import two commits from upstream

[markj]

Summary

6e474d8e3981 ("staging: vchiq_shim: avoid code duplication") refactors
some code which makes applying the subsequent patch easier.

49bec49fd7f2 ("staging: vc04_services: remove vchiq_copy_from_user")
addresses a user-triggerable integer overflow via the
VCHIQ_IOC_QUEUE_MESSAGE ioctl on /dev/vchiq (which has mode 0600 by
default). It also addresses insufficient validation of user-controlled
addresses in vchiq_copy_from_user().

Update the bcm2835_audio driver to follow the change to
vchi_msg_queue().

Reported by: Vicki Pfau

Test Plan

Compile-tested only

[markj]

@devesas.campos_gmail.com I wonder if you're able and willing to test this patch? I saw you've made some changes to vchiq recently; this patch was motivated by a report to FreeBSD's security team, but I don't have any hardware to test it with.

[adrian]

Maybe ask on freebsd-arm@ if anyone can test it out?

[markj]

In D58889#1351402, @adrian wrote:

Maybe ask on freebsd-arm@ if anyone can test it out?

Sure, done.

[devesas.campos_gmail.com]

Can try my best but I can't connect to my pi zero2 while it's running since all my usb-mini-to-usb-a adapters seem to have simultaneously decided go meet their maker so things might take me a while to do

Anyway, tried `cat </dev/random >/dev/dsp' with a patched kernel from yesterday and got a "panic _mtx_lock_sleep: recursed on non-recursive mutex dsp0.play.0 @ ../../../dev/sound/pcm/channel.c:590" . Backtrace shows:

…
mtx_lock_flags() at mtx_lock_flags+0x130
chn_intr() at chn_intr+0x30
bcmchan_trigger() at bcmchan_trigger+0x38
chn_trigger() at chn_trigger+0xd4
…

[markj]

In D58889#1351908, @devesas.campos_gmail.com wrote:

Can try my best but I can't connect to my pi zero2 while it's running since all my usb-mini-to-usb-a adapters seem to have simultaneously decided go meet their maker so things might take me a while to do

Anyway, tried `cat </dev/random >/dev/dsp' with a patched kernel from yesterday and got a "panic _mtx_lock_sleep: recursed on non-recursive mutex dsp0.play.0 @ ../../../dev/sound/pcm/channel.c:590" . Backtrace shows:

…
mtx_lock_flags() at mtx_lock_flags+0x130
chn_intr() at chn_intr+0x30
bcmchan_trigger() at bcmchan_trigger+0x38
chn_trigger() at chn_trigger+0xd4
…

This is probably an unrelated regression from commit 0794365d5d3cc93940b873553382288a2a2c88c4. @christos could you please take a look?

[christos]

bcmchan_trigger() calls chn_intr() which causes a lock recursion because chn_trigger() calls bcmchan_trigger() with the lock held already. However, chn_intr() is not meant to be called there in the CHANNEL_TRIGGER() method, so the code here is wrong in the first place. Does triggering work properly if the chn_intr() call is removed from bcmchan_trigger()? I see that it also calls bcm2835_worker_play_start(), which should be enough?

[devesas.campos_gmail.com]

In D58889#1352105, @christos wrote:

Does triggering work properly if the chn_intr() call is removed from bcmchan_trigger()?

Commented the line out and habemus noise.

Will try to do more extensive testing tomorrow.

[devesas.campos_gmail.com]

In D58889#1352129, @devesas.campos_gmail.com wrote:

Will try to do more extensive testing tomorrow.

Tested with the vchiq_test utility from rpi-userland in mode bulk, functional, ping and timer and they all succeeded.

[christos]

In D58889#1353814, @devesas.campos_gmail.com wrote:

In D58889#1352129, @devesas.campos_gmail.com wrote:

Will try to do more extensive testing tomorrow.

Tested with the vchiq_test utility from rpi-userland in mode bulk, functional, ping and timer and they all succeeded.

So the panic goes away and sound still works fine?

[adrian]

please at least test this with actual audio output (eg mpg123 with an mp3 into speakers.)

[devesas.campos_gmail.com]

In D58889#1353986, @christos wrote:

So the panic goes away and sound still works fine?

yep. didn't do extensive testing but played an internet radio station for a bit and it sounded fine to me.

I noticed that 95c04b2 also touched on bcm_audio and to be clear I have not tested it; only this patch with the call to chn_intr in bcmchan_trigger commented out.

[markj]

In D58889#1354350, @devesas.campos_gmail.com wrote:

In D58889#1353986, @christos wrote:

So the panic goes away and sound still works fine?

yep. didn't do extensive testing but played an internet radio station for a bit and it sounded fine to me.

I noticed that 95c04b2 also touched on bcm_audio and to be clear I have not tested it; only this patch with the call to chn_intr in bcmchan_trigger commented out.

Thanks for testing! I will commit this in the next day if there are no objections.

[christos]

@markj @devesas.campos_gmail.com D59055

[guest-seuros]

Tested on a Raspberry Pi 2 v1.1, applied on top of D59055 (chn_intr() recursion fix) since without it /dev/dsp panics independantly of this change.