Hi,
here is a simple patch to export the errno-symbol from the /lib/errno.c file.
This solves the problem with the soundcore module and works perfectly for
me...
Thomas Schlichter
P.S.: This patch is made against the 2.5.56 tree
Thomas Schlichter wrote:
>
> here is a simple patch to export the errno-symbol from the /lib/errno.c file.
> This solves the problem with the soundcore module and works perfectly for
> me...
Alternatively, one could simply reverse the one-line patch to
linux/sound/sound_firmware.c in the 2.5.55 patch set that broke things
to begin with.
--
-----------------------------------------------------------------------
Bob Tracy WTO + WIPO = DMCA? http://www.anti-dmca.org
[email protected]
-----------------------------------------------------------------------
On Mon, 2003-01-13 at 13:57, Thomas Schlichter wrote:
> Hi,
>
> here is a simple patch to export the errno-symbol from the /lib/errno.c file.
> This solves the problem with the soundcore module and works perfectly for
> me...
This actually shows a bug that has always been lurking. What if we load two
modules firmware at the same time. errno needs to be task private or we
perhaps need an errno_sem ?
On Mon, 13. Jan. 2003 16:13, Alan Cox wrote:
> This actually shows a bug that has always been lurking. What if we load two
> modules firmware at the same time. errno needs to be task private or we
> perhaps need an errno_sem ?
OK, I think I see the problem now!
But is soundcore the only place where 'errno' is used? Does this problem not
occur if any task modifies the errno value and an other one depends on its
previous value? I think this could happen even if no modules are used...
On 13 Jan 03 at 15:57, Thomas Schlichter wrote:
> On Mon, 13. Jan. 2003 16:13, Alan Cox wrote:
> > This actually shows a bug that has always been lurking. What if we load two
> > modules firmware at the same time. errno needs to be task private or we
> > perhaps need an errno_sem ?
>
> OK, I think I see the problem now!
> But is soundcore the only place where 'errno' is used? Does this problem not
> occur if any task modifies the errno value and an other one depends on its
> previous value? I think this could happen even if no modules are used...
There is no problem currently, because of nobody uses errno value at
all (in the firmware loader), it is just that inline functions generated
by syscallX() store error codes into errno...
Real problem is that firmware loader should use
filp_open/vfs_read/filp_close (or sys_open/sys_llseek/sys_read/sys_close if
you want to use fd interface, but filp_{open,close} and vfs_read are already
exported for modules while sys_open/sys_llseek/sys_read are not).
As an alternative, do_mod_firmware_load should be standalone userspace
program executed through call_usermodehelper or something like that...
Unfortunately we do not have an interface to distribute userspace binaries
together with kernel (except initrd) yet, so it would require either
adding do_mod_firmware_load into module-init-tools, or some simillar
package required by 2.[56].x kernels.
Also adding "#define errno (current()->exit_code)" at the beginning of
sound_firmware.c (just below #define __KERNEL_SYSCALLS__) should do
the trick, but I do not recommend taking this path.
Best regards,
Petr Vandrovec
[email protected]
On Mon, 13. Jan. 2003 17:33, Petr Vandrovec wrote:
> There is no problem currently, because of nobody uses errno value at
> all (in the firmware loader), it is just that inline functions generated
> by syscallX() store error codes into errno...
>
> Real problem is that firmware loader should use
> filp_open/vfs_read/filp_close (or sys_open/sys_llseek/sys_read/sys_close if
> you want to use fd interface, but filp_{open,close} and vfs_read are
> already exported for modules while sys_open/sys_llseek/sys_read are not).
>
> As an alternative, do_mod_firmware_load should be standalone userspace
> program executed through call_usermodehelper or something like that...
> Unfortunately we do not have an interface to distribute userspace binaries
> together with kernel (except initrd) yet, so it would require either
> adding do_mod_firmware_load into module-init-tools, or some simillar
> package required by 2.[56].x kernels.
>
> Also adding "#define errno (current()->exit_code)" at the beginning of
> sound_firmware.c (just below #define __KERNEL_SYSCALLS__) should do
> the trick, but I do not recommend taking this path.
> Best regards,
> Petr Vandrovec
> [email protected]
First of all a big THANKS!
Now, at least, I understood the real problem...
'errno' is only needed because open(), close(), llseek() and read() are used,
which do a syscall and the return code is written to a variable called
'errno'. This is NOT the 'errno' variable defined in lib/errno.c... OK.
And so the real problem is using these functions and as we ARE in the kernel
mode we do not really need the kernel traps...
So after realizing the real problem I think I am not the right person to fix
it. But if you want I could try...
Sincerely yours
Thomas Schlichter
>
> Now, at least, I understood the real problem...
> 'errno' is only needed because open(), close(), llseek() and read() are used,
> which do a syscall and the return code is written to a variable called
> 'errno'. This is NOT the 'errno' variable defined in lib/errno.c... OK.
>
> And so the real problem is using these functions and as we ARE in the kernel
> mode we do not really need the kernel traps...
Hi,
can someone with sound card requiring firmware download (OSS's trix,
sb_common, msnd_pinnacle, sscape, maui or pss) verify that code below
works? I know that it compiles without warnings, and looks obviously
correct (to me), but...
I think that I do not need any locking around reading i_size, as
nothing bad should happen if size changes, you'll get damaged firmware
upload at worst, and if you are updating firmware file while kernel
reads it, you deserve it (besides that vfs_read is one who will do final
check). And sys_stat (getattr) does not take any lock too, unless I
missed something.
Patch is for 2.5.57.
BTW, should I convert it to goto? It was really annoying that I had to
fix 3 error paths instead of one ;-)
Best regards,
Petr Vandrovec
[email protected]
diff -urdN linux/sound/sound_firmware.c linux/sound/sound_firmware.c
--- linux/sound/sound_firmware.c 2003-01-13 18:24:36.000000000 +0000
+++ linux/sound/sound_firmware.c 2003-01-13 18:45:12.000000000 +0000
@@ -9,39 +9,40 @@
static int do_mod_firmware_load(const char *fn, char **fp)
{
- int fd;
+ struct file* filp;
long l;
char *dp;
+ loff_t pos;
- fd = open(fn, 0, 0);
- if (fd == -1)
+ filp = filp_open(fn, 0, 0);
+ if (IS_ERR(filp))
{
printk(KERN_INFO "Unable to load '%s'.\n", fn);
return 0;
}
- l = lseek(fd, 0L, 2);
+ l = filp->f_dentry->d_inode->i_size;
if (l <= 0 || l > 131072)
{
printk(KERN_INFO "Invalid firmware '%s'\n", fn);
- sys_close(fd);
+ filp_close(filp, current->files);
return 0;
}
- lseek(fd, 0L, 0);
dp = vmalloc(l);
if (dp == NULL)
{
printk(KERN_INFO "Out of memory loading '%s'.\n", fn);
- sys_close(fd);
+ filp_close(filp, current->files);
return 0;
}
- if (read(fd, dp, l) != l)
+ pos = 0;
+ if (vfs_read(filp, dp, l, &pos) != l)
{
printk(KERN_INFO "Failed to read '%s'.\n", fn);
vfree(dp);
- sys_close(fd);
+ filp_close(filp, current->files);
return 0;
}
- close(fd);
+ filp_close(filp, current->files);
*fp = dp;
return (int) l;
}