Unhelpful permissions in some /proc files

Unhelpful permissions in some /proc files

[Guillaume Chazarain]

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

Hello,

Some tunables in /proc have a write() function, but as their
permission does not reflect it, it can be confusing to the user.

So here is a patch that corrects the mode of those files. Note that I
have only tested the "xrun_debug" entry.

Also, there is a little inconsistency in that some code does
'entry->mode = S_IFREG | S_IRUGO | S_IWUSR;' whereas some other does
'entry->mode |= S_IWUSR;'.

Kind regards.

-- 
Guillaume


[-- Attachment #2: alsa_proc_mode.diff --]
[-- Type: text/x-patch, Size: 4378 bytes --]

--- linux-2.6.12-rc1-bk3/sound/core/pcm.c
+++ linux-2.6.12-rc1-bk3-new/sound/core/pcm.c
@@ -447,6 +447,7 @@ static int snd_pcm_stream_proc_init(snd_
 
 #ifdef CONFIG_SND_DEBUG
 	if ((entry = snd_info_create_card_entry(pcm->card, "xrun_debug", pstr->proc_root)) != NULL) {
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.read_size = 64;
 		entry->c.text.read = snd_pcm_xrun_debug_read;
 		entry->c.text.write_size = 64;
--- linux-2.6.12-rc1-bk3/sound/core/pcm_memory.c
+++ linux-2.6.12-rc1-bk3-new/sound/core/pcm_memory.c
@@ -200,6 +200,7 @@ static int snd_pcm_lib_preallocate_pages
 		substream->buffer_bytes_max = substream->dma_buffer.bytes;
 	substream->dma_max = max;
 	if ((entry = snd_info_create_card_entry(substream->pcm->card, "prealloc", substream->proc_root)) != NULL) {
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.read_size = 64;
 		entry->c.text.read = snd_pcm_lib_preallocate_proc_read;
 		entry->c.text.write_size = 64;
--- linux-2.6.12-rc1-bk3/sound/drivers/opl4/opl4_proc.c
+++ linux-2.6.12-rc1-bk3-new/sound/drivers/opl4/opl4_proc.c
@@ -145,6 +145,7 @@ int snd_opl4_create_proc(opl4_t *opl4)
 			entry->size = 1 * 1024 * 1024;
 		}
 		entry->content = SNDRV_INFO_CONTENT_DATA;
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.ops = &snd_opl4_mem_proc_ops;
 		entry->module = THIS_MODULE;
 		entry->private_data = opl4;
--- linux-2.6.12-rc1-bk3/sound/pci/ca0106/ca0106_proc.c
+++ linux-2.6.12-rc1-bk3-new/sound/pci/ca0106/ca0106_proc.c
@@ -416,6 +416,7 @@ int __devinit snd_ca0106_proc_init(ca010
 		snd_info_set_text_ops(entry, emu, 1024, snd_ca0106_proc_iec958);
 	if(! snd_card_proc_new(emu->card, "ca0106_reg32", &entry)) {
 		snd_info_set_text_ops(entry, emu, 1024, snd_ca0106_proc_reg_read32);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_ca0106_proc_reg_write32;
 	}
@@ -425,6 +426,7 @@ int __devinit snd_ca0106_proc_init(ca010
 		snd_info_set_text_ops(entry, emu, 1024, snd_ca0106_proc_reg_read8);
 	if(! snd_card_proc_new(emu->card, "ca0106_regs1", &entry)) {
 		snd_info_set_text_ops(entry, emu, 1024, snd_ca0106_proc_reg_read1);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_ca0106_proc_reg_write;
 //		entry->private_data = emu;
--- linux-2.6.12-rc1-bk3/sound/pci/emu10k1/emu10k1x.c
+++ linux-2.6.12-rc1-bk3-new/sound/pci/emu10k1/emu10k1x.c
@@ -1073,6 +1073,7 @@ static int __devinit snd_emu10k1x_proc_i
 	
 	if(! snd_card_proc_new(emu->card, "emu10k1x_regs", &entry)) {
 		snd_info_set_text_ops(entry, emu, 1024, snd_emu10k1x_proc_reg_read);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu10k1x_proc_reg_write;
 		entry->private_data = emu;
--- linux-2.6.12-rc1-bk3/sound/pci/emu10k1/emuproc.c
+++ linux-2.6.12-rc1-bk3-new/sound/pci/emu10k1/emuproc.c
@@ -498,26 +498,31 @@ int __devinit snd_emu10k1_proc_init(emu1
 #ifdef CONFIG_SND_DEBUG
 	if (! snd_card_proc_new(emu->card, "io_regs", &entry)) {
 		snd_info_set_text_ops(entry, emu, 1024, snd_emu_proc_io_reg_read);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu_proc_io_reg_write;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs00a", &entry)) {
 		snd_info_set_text_ops(entry, emu, 65536, snd_emu_proc_ptr_reg_read00a);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu_proc_ptr_reg_write00;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs00b", &entry)) {
 		snd_info_set_text_ops(entry, emu, 65536, snd_emu_proc_ptr_reg_read00b);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu_proc_ptr_reg_write00;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs20a", &entry)) {
 		snd_info_set_text_ops(entry, emu, 65536, snd_emu_proc_ptr_reg_read20a);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu_proc_ptr_reg_write20;
 	}
 	if (! snd_card_proc_new(emu->card, "ptr_regs20b", &entry)) {
 		snd_info_set_text_ops(entry, emu, 65536, snd_emu_proc_ptr_reg_read20b);
+		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
 		entry->c.text.write_size = 64;
 		entry->c.text.write = snd_emu_proc_ptr_reg_write20;
 	}

Re: Unhelpful permissions in some /proc files

[Takashi Iwai]

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

At Fri, 01 Apr 2005 23:19:37 +0200,
Guillaume Chazarain wrote:
> 
> Hello,
> 
> Some tunables in /proc have a write() function, but as their
> permission does not reflect it, it can be confusing to the user.
> 
> So here is a patch that corrects the mode of those files. Note that I
> have only tested the "xrun_debug" entry.

Thanks for the patch.

> Also, there is a little inconsistency in that some code does
> 'entry->mode = S_IFREG | S_IRUGO | S_IWUSR;' whereas some other does
> 'entry->mode |= S_IWUSR;'.

I think the latter form is enough.

BTW, instead of patching each part, checking S_IWUSR in
snd_info_register() may an easier solution.  The patch is below.


Takashi

[-- Attachment #2: Type: text/plain, Size: 800 bytes --]

Index: alsa-kernel/core/info.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/core/info.c,v
retrieving revision 1.48
diff -u -r1.48 info.c
--- alsa-kernel/core/info.c	22 Mar 2005 15:18:27 -0000	1.48
+++ alsa-kernel/core/info.c	4 Apr 2005 10:31:14 -0000
@@ -910,6 +910,12 @@
 
 	snd_assert(entry != NULL, return -ENXIO);
 	root = entry->parent == NULL ? snd_proc_root : entry->parent->p;
+	if ((entry->content == SNDRV_INFO_CONTENT_TEXT && entry->c.text.write_size) ||
+	    (entry->content == SNDRV_INFO_CONTENT_DATA && entry->c.ops->write)) {
+		/* fix up write permission */
+		if (! (entry->mode & S_IWUGO))
+			entry->mode |= S_IWUSR;
+	}
 	down(&info_mutex);
 	p = snd_create_proc_entry(entry->name, entry->mode, root);
 	if (!p) {

Re: Unhelpful permissions in some /proc files

[Guillaume Chazarain]

Takashi Iwai wrote:

>BTW, instead of patching each part, checking S_IWUSR in
>snd_info_register() may an easier solution.  The patch is below.
>  
>
Yes, of course, but I didn't like the fact that it looked like a fix 
done by the callee.
Either snd_info_register() is passed good permissions and it doesn't 
change them,
or its job to set them, then they should not be set before.

The comment in your patch explained exactly what I felt, but anyway, if 
you like
it that way, it's fine for me too.

>+		/* fix up write permission */
>
-- 
Guillaume



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: Unhelpful permissions in some /proc files

[Takashi Iwai]

At Mon, 04 Apr 2005 14:24:44 +0200,
Guillaume Chazarain wrote:
> 
> Takashi Iwai wrote:
> 
> >BTW, instead of patching each part, checking S_IWUSR in
> >snd_info_register() may an easier solution.  The patch is below.
> >  
> >
> Yes, of course, but I didn't like the fact that it looked like a fix 
> done by the callee.
> Either snd_info_register() is passed good permissions and it doesn't 
> change them,
> or its job to set them, then they should not be set before.
> 
> The comment in your patch explained exactly what I felt, but anyway, if 
> you like
> it that way, it's fine for me too.

Sorry for the late response.  I forgot this one.

Well, since the changes are not in many places anyway, I just added
the permission in each place as you did.  Now it's fixed on CVS.


Thanks,

Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click