Bug 156433

Summary:

[sound] [patch] OSS4/VPC is broken on 64-bit platforms

Product:

Base System

Reporter:

Grigori Goronzy <greg>

Component:

kern

Assignee:

Andriy Gapon <avg>

Status:

Closed FIXED

Severity:

Affects Only Me

Priority:

Normal

Version:

Unspecified

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
file.diff	none
test_vol.c	none

Description Grigori Goronzy 2011-04-15 22:40:07 UTC

The OSS4 API for volume control is broken on 64-bit platforms. I tracked it down and noticed that the ioctl passthrough/wrapper uses "int" where it should use an "u_long" for passing ioctl parameters. On most 32-bit platforms this is not an issue, as sizeof(int)==sizeof(u_long). Not so on 64-bit platforms, though.

This boils down to a signed vs unsigned issue when the parameter is converted from int to u_long. This affects the SNDCTL_DSP_SETPLAYVOL ioctl and possibly others.

Fix: The attached patch.

Patch attached with submission follows:
How-To-Repeat: 1. Install mplayer2
2. Play an audio file through mplayer2's OSS output (which uses the aforementioned API call)
3. Try to change volume with "9" and "0" keys
4. Nothing happens

Comment 1 Mark Linimon freebsd_committer

2011-04-17 02:12:11 UTC

Responsible Changed
From-To: freebsd-bugs->freebsd-multimedia

Over to maintainer(s).

Comment 2 Test Rat 2011-07-29 01:25:37 UTC

Any chance the fix will be committed before 9.0-RELEASE? The ioctl is
used by more apps than just mplayer2, see

  http://google.com/codesearch?q=SNDCTL_DSP_SETPLAYVOL
  (gst-plugins-good, wine, xmms2, moc, qmmp, etc)

I've tried to change volume with attached test program but it stays the same.
This is also reflected in /dev/sndstat, try

  http://people.freebsd.org/~ariff/utils/appsmixer

Comment 3 dfilter service freebsd_committer

2011-09-12 09:38:35 UTC

Author: avg
Date: Mon Sep 12 08:38:21 2011
New Revision: 225505
URL: http://svn.freebsd.org/changeset/base/225505

Log:
  dsp_ioctl: fix type of variable used to store ioctl request
  
  PR:		kern/156433
  Submitted by:	Grigori Goronzy <greg@chown.ath.cx>
  Reviewed by:	hselasky
  Approved by:	re (kib)
  MFC after:	1 week

Modified:
  head/sys/dev/sound/pcm/dsp.c

Modified: head/sys/dev/sound/pcm/dsp.c
==============================================================================
--- head/sys/dev/sound/pcm/dsp.c	Mon Sep 12 06:41:13 2011	(r225504)
+++ head/sys/dev/sound/pcm/dsp.c	Mon Sep 12 08:38:21 2011	(r225505)
@@ -1062,7 +1062,8 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd
 {
     	struct pcm_channel *chn, *rdch, *wrch;
 	struct snddev_info *d;
-	int *arg_i, ret, tmp, xcmd;
+	u_long xcmd;
+	int *arg_i, ret, tmp;
 
 	d = dsp_get_info(i_dev);
 	if (!DSP_REGISTERED(d, i_dev))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 4 Andriy Gapon freebsd_committer

2011-09-12 09:42:41 UTC

State Changed
From-To: open->patched

A variant of the patch is committed to head.

Comment 5 Andriy Gapon freebsd_committer

2011-09-12 09:42:41 UTC

Responsible Changed
From-To: freebsd-multimedia->avg

Take.

Comment 6 dfilter service freebsd_committer

2011-11-05 11:04:45 UTC

Author: avg
Date: Sat Nov  5 11:04:25 2011
New Revision: 227106
URL: http://svn.freebsd.org/changeset/base/227106

Log:
  MFC r225505: dsp_ioctl: fix type of variable used to store ioctl request
  
  PR:		kern/156433

Modified:
  stable/8/sys/dev/sound/pcm/dsp.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/dev/sound/pcm/dsp.c
==============================================================================
--- stable/8/sys/dev/sound/pcm/dsp.c	Sat Nov  5 10:00:29 2011	(r227105)
+++ stable/8/sys/dev/sound/pcm/dsp.c	Sat Nov  5 11:04:25 2011	(r227106)
@@ -1062,7 +1062,8 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd
 {
     	struct pcm_channel *chn, *rdch, *wrch;
 	struct snddev_info *d;
-	int *arg_i, ret, tmp, xcmd;
+	u_long xcmd;
+	int *arg_i, ret, tmp;
 
 	d = dsp_get_info(i_dev);
 	if (!DSP_REGISTERED(d, i_dev))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 7 dfilter service freebsd_committer

2011-11-05 11:18:59 UTC

Author: avg
Date: Sat Nov  5 11:18:46 2011
New Revision: 227107
URL: http://svn.freebsd.org/changeset/base/227107

Log:
  MFC r225505: dsp_ioctl: fix type of variable used to store ioctl request
  
  PR:		kern/156433

Modified:
  stable/7/sys/dev/sound/pcm/dsp.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/dev/sound/pcm/dsp.c
==============================================================================
--- stable/7/sys/dev/sound/pcm/dsp.c	Sat Nov  5 11:04:25 2011	(r227106)
+++ stable/7/sys/dev/sound/pcm/dsp.c	Sat Nov  5 11:18:46 2011	(r227107)
@@ -778,7 +778,8 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd
 {
     	struct pcm_channel *chn, *rdch, *wrch;
 	struct snddev_info *d;
-	int *arg_i, ret, kill, tmp, xcmd;
+	u_long xcmd;
+	int *arg_i, ret, kill, tmp;
 
 	d = dsp_get_info(i_dev);
 	if (!DSP_REGISTERED(d, i_dev))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

Comment 8 Andriy Gapon freebsd_committer

2011-11-05 11:20:21 UTC

State Changed
From-To: patched->closed

The fix is MFCed.