Skip to contentSkip to conversation

Unhelpful permissions in some /proc files

PATCH Guillaume Chazarain 2 participants 4 messages Apr 1, 2005 – Apr 12, 2005

Subsystems (2)
Sound OPL4 driver
lore
Share & tools
lore

Guillaume Chazarain

author
#0 lore raw

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.

Signature · 1 line
Guillaume
sound/core/pcm.c +1 -0
@@ -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;
sound/core/pcm_memory.c +1 -0
@@ -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;
sound/drivers/opl4/opl4_proc.c +1 -0
@@ -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;
sound/pci/ca0106/ca0106_proc.c +2 -0
@@ -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;
sound/pci/emu10k1/emu10k1x.c +1 -0
@@ -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;
sound/pci/emu10k1/emuproc.c +5 -0
@@ -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;
 	}
#1 lore raw

At Fri, 01 Apr 2005 23:19:37 +0200,

Quoted 8 lines · Guillaume Chazarain

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.

Quoted 3 lines

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

Index: alsa-kernel/core/info.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/core/info.c,v retrieving revision 1.48

core/info.c +6 -0
diff -u -r1.48 info.c
@@ -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) {
author
#2 lore raw
Quoted 4 lines · Takashi Iwai

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.

Quoted 2 lines
core/info.c +1 -0
+		/* fix up write permission */
Signature · 9 lines
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
#3 lore raw

At Mon, 04 Apr 2005 14:24:44 +0200,

Quoted 13 lines · Guillaume Chazarain
Quoted 4 lines · Takashi Iwai

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.

Quoted 3 lines

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