[PATCH] Re: Segmentation fault in i810_audio.c:__i810_update_lvi

[Gary Wong <gtw@cs.bu.edu>]

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

On Tue, May 11, 2004 at 12:27:28AM -0700, Andrew Morton wrote:
> Gary Wong <gtw@cs.bu.edu> wrote:
> > I believe that one of two fixes should be applied: either the
> >  SNDCTL_DSP_SETTRIGGER ioctl handling should not enable the
> >  PCM_ENABLE_{IN,OUT}PUT bits unless file->f_mode is compatible,
> >  or i810_release() should ignore the PCM_ENABLE_* bits without
> >  the corresponding FMODE_*.
> 
> The first option sounds more appropriate but I wonder if it could break
> existing applications?  Probably not, if it oopses.

I agree; I had a look at a few other drivers at random (audio, esssolo1,
trident) and they all silently ignore inappropriate PCM_ENABLE_* bits.

> Let's try option #1, please.

OK, a patch is attached.  It seems to fix the problem on a 2.6.3 system
with Herbert Xu's patches from Jeff Garzik applied.

break-i810.c is a small test case which hopefully demonstrates the
problem.

Cheers,
Gary.
-- 
     Gary Wong          gtw@cs.bu.edu          http://cs-people.bu.edu/gtw/

[-- Attachment #2: i810-settrigger-patch --]
[-- Type: text/plain, Size: 1260 bytes --]

--- i810_audio.c.herbert	2004-05-11 12:36:05.674715334 -0400
+++ i810_audio.c	2004-05-11 12:41:50.101624990 -0400
@@ -2186,6 +2186,10 @@
 #if defined(DEBUG) || defined(DEBUG_MMAP)
 		printk("SNDCTL_DSP_SETTRIGGER 0x%x\n", val);
 #endif
+		if (!(file->f_mode & FMODE_READ ))
+			val &= ~PCM_ENABLE_INPUT;
+		if (!(file->f_mode & FMODE_WRITE ))
+			val &= ~PCM_ENABLE_OUTPUT;
 		if((file->f_mode & FMODE_READ) && !(val & PCM_ENABLE_INPUT) && dmabuf->enable == ADC_RUNNING) {
 			stop_adc(state);
 		}
@@ -2193,7 +2197,7 @@
 			stop_dac(state);
 		}
 		dmabuf->trigger = val;
-		if((file->f_mode & FMODE_WRITE) && (val & PCM_ENABLE_OUTPUT) && !(dmabuf->enable & DAC_RUNNING)) {
+		if((val & PCM_ENABLE_OUTPUT) && !(dmabuf->enable & DAC_RUNNING)) {
 			if (!dmabuf->write_channel) {
 				dmabuf->ready = 0;
 				dmabuf->write_channel = state->card->alloc_pcm_channel(state->card);
@@ -2214,7 +2218,7 @@
 			i810_update_lvi(state, 0);
 			start_dac(state);
 		}
-		if((file->f_mode & FMODE_READ) && (val & PCM_ENABLE_INPUT) && !(dmabuf->enable & ADC_RUNNING)) {
+		if((val & PCM_ENABLE_INPUT) && !(dmabuf->enable & ADC_RUNNING)) {
 			if (!dmabuf->read_channel) {
 				dmabuf->ready = 0;
 				dmabuf->read_channel = state->card->alloc_rec_pcm_channel(state->card);

[-- Attachment #3: break-i810.c --]
[-- Type: text/plain, Size: 891 bytes --]

#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/soundcard.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

extern int main( void ) {

#define DSP "/dev/dsp"
    
    int f = open( DSP, O_RDONLY );
    int i, n;
    fd_set fds;
    
    if( f < 0 ) {
	perror( DSP );

	return EXIT_FAILURE;
    }

    n = 0;
    if( ioctl( f, SNDCTL_DSP_SETTRIGGER, &n ) < 0 ) {
	perror( "SNDCTL_DSP_SETTRIGGER" );

	return EXIT_FAILURE;
    }

    n = PCM_ENABLE_INPUT | PCM_ENABLE_OUTPUT;
    if( ioctl( f, SNDCTL_DSP_SETTRIGGER, &n ) < 0 ) {
	perror( "SNDCTL_DSP_SETTRIGGER" );

	return EXIT_FAILURE;
    }

    for( i = 0; i < 10; i++ ) {
	char a[ 1024 ];
	
	FD_ZERO( &fds );
	FD_SET( f, &fds );

	if( select( f + 1, &fds, NULL, NULL, NULL ) < 0 )
	    perror( "select" );
	
	if( read( f, a, 1024 ) < 0 )
	    perror( "read" );
    }
    
    close( f );

    return EXIT_SUCCESS;
}