tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Audio update pertaining to mixer devices and hw.driverN.multiuser



Hi,

I have attached a patch that moves the security credendials that are checked 
on an open of a mixer/audio/audioctl device to the particuar audio channel 
instead of a device global.

The way the check operation is performed on an audio open is that the 
credentials are compared to the first in the list of playing channels.

User noticable changes are that it is not permissible to open /dev/mixer if 
another user has audio playing - so it is not possible to interfere with 
another users mixer settings.

The super user has access to audio/audioctl/mixer devices at all times.

These changes primarily affect audio when hw.driverN.multiuser=0 although it is 
no longer permissible to interfere with the audio_info structures of an audio 
channel belonging to another user regardless of the value of 
hw.driverN.multiuser.

This should address PR kern/52627 audio_setchan affecting privileged process.

If there are no obections, I'd like to commit this change by this time next 
week.

Best regards,

Nat
Index: sys/kauth.h
===================================================================
RCS file: /cvsroot/src/sys/sys/kauth.h,v
retrieving revision 1.76
diff -u -p -r1.76 kauth.h
--- sys/kauth.h	26 Apr 2018 18:54:09 -0000	1.76
+++ sys/kauth.h	24 May 2018 07:13:15 -0000
@@ -346,6 +346,7 @@ enum {
 	KAUTH_DEVICE_TTY_VIRTUAL,
 	KAUTH_DEVICE_WSCONS_KEYBOARD_BELL,
 	KAUTH_DEVICE_WSCONS_KEYBOARD_KEYREPEAT,
+	KAUTH_DEVICE_AUDIO_ALLOW,
 };
 
 /*
Index: secmodel/suser/secmodel_suser.c
===================================================================
RCS file: /cvsroot/src/sys/secmodel/suser/secmodel_suser.c,v
retrieving revision 1.44
diff -u -p -r1.44 secmodel_suser.c
--- secmodel/suser/secmodel_suser.c	26 Apr 2018 18:54:09 -0000	1.44
+++ secmodel/suser/secmodel_suser.c	24 May 2018 07:13:15 -0000
@@ -884,6 +884,7 @@ secmodel_suser_device_cb(kauth_cred_t cr
 	result = KAUTH_RESULT_DEFER;
 
 	switch (action) {
+	case KAUTH_DEVICE_AUDIO_ALLOW:
 	case KAUTH_DEVICE_BLUETOOTH_SETPRIV:
 	case KAUTH_DEVICE_BLUETOOTH_SEND:
 	case KAUTH_DEVICE_BLUETOOTH_RECV:
Index: dev/audiovar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/audiovar.h,v
retrieving revision 1.68
diff -u -p -r1.68 audiovar.h
--- dev/audiovar.h	15 Nov 2017 04:28:45 -0000	1.68
+++ dev/audiovar.h	24 May 2018 07:13:15 -0000
@@ -116,6 +116,7 @@ struct audio_chan {
 	struct virtual_channel	*vc;
 	int	chan;			/* virtual channel */
 	int	deschan;		/* desired channel for ioctls*/
+	kauth_cred_t	sc_credentials;	/* audio channel user's credentials */
 	SIMPLEQ_ENTRY(audio_chan) entries;
 };
 
@@ -293,7 +294,6 @@ struct audio_softc {
 	struct audio_params sc_vchan_params;
 
 	bool		sc_multiuser;
-	kauth_cred_t	sc_credentials;		/* audio user's credentials */
 };
 
 #endif /* _SYS_DEV_AUDIOVAR_H_ */
Index: dev/audio.c
===================================================================
RCS file: /cvsroot/src/sys/dev/audio.c,v
retrieving revision 1.457
diff -u -p -r1.457 audio.c
--- dev/audio.c	22 May 2018 01:35:49 -0000	1.457
+++ dev/audio.c	24 May 2018 07:13:16 -0000
@@ -299,6 +299,7 @@ void	audio_clear_intr_unlocked(struct au
 int	audio_alloc_ring(struct audio_softc *, struct audio_ringbuffer *, int,
 			 size_t);
 void	audio_free_ring(struct audio_softc *, struct audio_ringbuffer *);
+static void audio_free_chan(struct audio_chan *);
 static int audio_setup_pfilters(struct audio_softc *, const audio_params_t *,
 			      stream_filter_list_t *, struct virtual_channel *);
 static int audio_setup_rfilters(struct audio_softc *, const audio_params_t *,
@@ -364,6 +365,12 @@ static int audiopoll(struct file *, int)
 static int audiokqfilter(struct file *, struct knote *);
 static int audiostat(struct file *, struct stat *);
 
+static int audio_access_check_set(struct audio_softc *, struct audio_chan *);
+static int audio_listener_cb(kauth_cred_t, kauth_action_t, void *,
+			     void *, void *, void *, void *);
+
+static kauth_listener_t	audio_listener;
+
 struct portname {
 	const char *name;
 	int mask;
@@ -466,6 +473,29 @@ int auto_config_channels[] = { 2, AUDIO_
 int auto_config_freq[] = { 48000, 44100, 96000, 192000, 32000,
 			   22050, 16000, 11025, 8000, 4000 };
 
+/*
+ * kauth listener to check for super user or owner of vchan.
+ */
+static int
+audio_listener_cb(kauth_cred_t cred, kauth_action_t action, void *cookie,
+    void *arg0, void *arg1, void *arg2, void *arg3)
+{
+	int result = KAUTH_RESULT_DEFER;
+	struct audio_chan *chan = (struct audio_chan *)arg1;
+
+	if (action != KAUTH_DEVICE_AUDIO_ALLOW)
+		return result;
+
+	if (chan == NULL)
+		return result;
+
+	if (kauth_cred_geteuid(cred) ==
+	    kauth_cred_geteuid(chan->sc_credentials))
+		result = KAUTH_RESULT_ALLOW;
+
+	return result;
+}
+
 CFATTACH_DECL3_NEW(audio, sizeof(struct audio_softc),
     audiomatch, audioattach, audiodetach, audioactivate, audiorescan,
     audiochilddet, DVF_DETACH_SHUTDOWN);
@@ -1282,6 +1312,15 @@ audio_free_ring(struct audio_softc *sc, 
 	r->s.start = NULL;
 }
 
+static void
+audio_free_chan(struct audio_chan *chan)
+{
+	if (chan->sc_credentials != NULL)
+		kauth_cred_free(chan->sc_credentials);
+
+	kmem_free(chan, sizeof(struct audio_chan));
+}
+
 static int
 audio_setup_pfilters(struct audio_softc *sc, const audio_params_t *pp,
 		     stream_filter_list_t *pfilters, struct virtual_channel *vc)
@@ -1556,6 +1595,27 @@ stream_filter_list_prepend(stream_filter
 	list->req_size++;
 }
 
+static int
+audio_access_check_set(struct audio_softc *sc, struct audio_chan *chan)
+{
+	struct audio_chan *kchan;
+
+	kauth_cred_t currcred = kauth_cred_get();
+	if (sc->sc_usemixer && sc->sc_multiuser == false) {
+		kchan = SIMPLEQ_FIRST(&sc->sc_audiochan);
+		if (kchan != NULL && kauth_authorize_device(currcred,
+		    KAUTH_DEVICE_AUDIO_ALLOW, sc, kchan, NULL, NULL))
+			return EPERM;
+	}
+	
+	KASSERT(chan->sc_credentials == NULL);
+
+	chan->sc_credentials = currcred;
+	kauth_cred_hold(chan->sc_credentials);
+
+	return 0;
+}
+
 /*
  * Look up audio device and acquire locks for device access.
  */
@@ -1731,7 +1791,7 @@ audioclose(struct file *fp)
 		break;
 	}
 	if (error == 0) {
-		kmem_free(fp->f_audioctx, sizeof(struct audio_chan));
+		audio_free_chan(chan);
 		fp->f_audioctx = NULL;
 	}
 
@@ -2175,6 +2235,10 @@ audioctl_open(dev_t dev, struct audio_so
 		vc = sc->sc_hwvc;
 	chan->vc = vc;
 
+	error = audio_access_check_set(sc, chan);
+	if (error)
+		goto bad;
+
 	error = fd_allocfile(&fp, &fd);
 	if (error)
 		goto bad;
@@ -2189,7 +2253,7 @@ audioctl_open(dev_t dev, struct audio_so
 	*nfp = fp;
 	return error;
 bad:
-	kmem_free(chan, sizeof(struct audio_chan));
+	audio_free_chan(chan);
 	return error;
 }
 
@@ -2261,9 +2325,11 @@ audio_open(dev_t dev, struct audio_softc
 			goto bad;
 	}
 
+	error = audio_access_check_set(sc, chan);
+	if (error)
+		goto bad;
+
 	if (!sc->sc_usemixer || sc->sc_opens + sc->sc_recopens == 0) {
-		sc->sc_credentials = kauth_cred_get();
-		kauth_cred_hold(sc->sc_credentials);
 		if (hw->open != NULL) {
 			mutex_enter(sc->sc_intr_lock);
 			error = hw->open(sc->hw_hdl, flags);
@@ -2285,15 +2351,6 @@ audio_open(dev_t dev, struct audio_softc
 		sc->sc_eof = 0;
 		vc->sc_rbus = false;
 		sc->sc_async_audio = 0;
-	} else if (sc->sc_multiuser == false) {
-		/* XXX:NS Should be handled correctly. */
-		/* Do we allow multi user access */
-		if (kauth_cred_geteuid(sc->sc_credentials) !=
-		    kauth_cred_geteuid(kauth_cred_get()) &&
-		    kauth_cred_geteuid(kauth_cred_get()) != 0) {
-			error = EPERM;
-			goto bad;
-		}
 	}
 
 	mutex_enter(sc->sc_intr_lock);
@@ -2381,7 +2438,7 @@ bad:
 	} else
 		mutex_enter(sc->sc_lock);
 
-	kmem_free(chan, sizeof(struct audio_chan));
+	audio_free_chan(chan);
 	return error;
 }
 
@@ -2582,7 +2639,6 @@ audio_close(struct audio_softc *sc, int 
 
 	if (sc->sc_opens + sc->sc_recopens == 1) {
 		sc->sc_async_audio = 0;
-		kauth_cred_free(sc->sc_credentials);
 	}
 
 	vc->sc_open = 0;
@@ -3126,6 +3182,12 @@ audio_ioctl(dev_t dev, struct audio_soft
 	} else
 		pchan = chan;
 
+	if (sc->sc_usemixer) {
+		if (kauth_authorize_device(kauth_cred_get(),
+		    KAUTH_DEVICE_AUDIO_ALLOW, sc, pchan, NULL, NULL))
+			return EPERM;
+	}
+
 	if (!sc->sc_usemixer || chan->deschan != 0)
 		vc = pchan->vc;
 	else
@@ -5056,18 +5118,25 @@ mixer_open(dev_t dev, struct audio_softc
 
 	DPRINTF(("mixer_open: flags=0x%x sc=%p\n", flags, sc));
 
-	error = fd_allocfile(&fp, &fd);
-	if (error)
-		return error;
-
 	chan = kmem_zalloc(sizeof(struct audio_chan), KM_SLEEP);
 	chan->dev = dev;
 
+	error = audio_access_check_set(sc, chan);
+	if (error)
+		goto bad;
+
+	error = fd_allocfile(&fp, &fd);
+	if (error)
+		goto bad;
+
 	error = fd_clone(fp, fd, flags, &audio_fileops, chan);
 	KASSERT(error == EMOVEFD);
 
 	*nfp = fp;
 	return error;
+bad:
+	audio_free_chan(chan);
+	return error;
 }
 
 /*
@@ -6374,9 +6443,9 @@ audio_modcmd(modcmd_t cmd, void *arg)
 {
 	int error = 0;
 
-#ifdef _MODULE
 	switch (cmd) {
 	case MODULE_CMD_INIT:
+#ifdef _MODULE
 		error = devsw_attach(audio_cd.cd_name, NULL, &audio_bmajor,
 		    &audio_cdevsw, &audio_cmajor);
 		if (error)
@@ -6387,20 +6456,32 @@ audio_modcmd(modcmd_t cmd, void *arg)
 		if (error) {
 			devsw_detach(NULL, &audio_cdevsw);
 		}
+#endif
+		if (!error) {
+			audio_listener = kauth_listen_scope(KAUTH_SCOPE_DEVICE,
+			    audio_listener_cb, NULL);
+		}
 		break;
 	case MODULE_CMD_FINI:
+#ifdef _MODULE
 		devsw_detach(NULL, &audio_cdevsw);
 		error = config_fini_component(cfdriver_ioconf_audio,
 		   cfattach_ioconf_audio, cfdata_ioconf_audio);
 		if (error)
 			devsw_attach(audio_cd.cd_name, NULL, &audio_bmajor,
 			    &audio_cdevsw, &audio_cmajor);
+#endif
+		if (audio_listener) {
+			kauth_unlisten_scope(audio_listener);
+			audio_listener = NULL;
+		}
 		break;
 	default:
+#ifdef _MODULE
 		error = ENOTTY;
+#endif
 		break;
 	}
-#endif
 
 	return error;
 }


Home | Main Index | Thread Index | Old Index