-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi.
I'm using kernel 2.6.10 from kernel.org, on an U10 sun ultrasparcIIi.
I'm playing a bit with openprom settings using openpromfs, and I wasn't
able to change the oem-logo. Here is the command and the oops message :
# cat /some/logo/file > /proc/opemprom/options/oem-logo
(same if openpromfs is mounted in /mnt/openprom for example)
Jan 3 12:54:06 usparc kernel: cat(2323): Oops [#1]
Jan 3 12:54:06 usparc kernel: TSTATE: 0000004411009600 TPC: 00000000020520c4 TNPC: 00000000020520c8 Y: 00000000 Tainted: P
Jan 3 12:54:06 usparc kernel: TPC: <property_read+0x4/0xaa0 [openpromfs]>
Jan 3 12:54:06 usparc kernel: g0: 0000000000000000 g1: fffff800076b41e0 g2: 0000000000fffffe g3: 0000000000000000
Jan 3 12:54:06 usparc kernel: g4: fffff800075cd6e0 g5: fffff800076b41e0 g6: fffff80006978000 g7: 0000000070015638
Jan 3 12:54:06 usparc kernel: o0: 00000000000248a0 o1: 0000000000002000 o2: 00000000effff920 o3: 0000000000000000
Jan 3 12:54:06 usparc kernel: o4: 0000000000000c00 o5: 0000000000000c00 sp: fffff8000697b2e1 ret_pc: 0000000000011654
Jan 3 12:54:06 usparc kernel: RPC: <0x11654>
Jan 3 12:54:06 usparc kernel: l0: 0000000000000001 l1: 0000000000000010 l2: 000000002e300047 l3: 0000000000470000
Jan 3 12:54:06 usparc kernel: l4: 000042435f322e30 l5: 0000000000000001 l6: 0000000000000000 l7: 0000000000023800
Jan 3 12:54:06 usparc kernel: i0: fffff800076b41e0 i1: 0000000000000000 i2: 0000000000000000 i3: 0000000000000000
Jan 3 12:54:06 usparc kernel: i4: 0000000000474c49 i5: 0000000000000047 i6: fffff8000697b411 i7: 000000000205340c
Jan 3 12:54:06 usparc kernel: I7: <property_write+0x8ac/0x8e0 [openpromfs]>
Jan 3 12:54:06 usparc kernel: Caller[000000000205340c]: property_write+0x8ac/0x8e0 [openpromfs]
Jan 3 12:54:06 usparc kernel: Caller[000000000048c548]: vfs_write+0xa8/0x100
Jan 3 12:54:06 usparc kernel: Caller[000000000048c62c]: sys_write+0x2c/0x60
Jan 3 12:54:06 usparc kernel: Caller[0000000000411174]: linux_sparc_syscall32+0x34/0x40
Jan 3 12:54:06 usparc kernel: Caller[0000000000012860]: 0x12860
Jan 3 12:54:06 usparc kernel: Instruction DUMP: 01000000 01000000 9de3bed0 <ca5ec000> 03003fff 84102000 821063fe c65e2010 a8100018
The problem is here in the source file:
Line numbers may not be accurate as I tried some changes.
(line 86)
static ssize_t property_read(struct file *filp, char __user *buf,
size_t count, loff_t *ppos)
{
[...]
if (*ppos >= 0xffffff || count >= 0xffffff)
return -EINVAL;
(line 321)
static ssize_t property_write(struct file *filp, const char __user *buf,
size_t count, loff_t *ppos)
{
[...]
if (!filp->private_data) {
i = property_read (filp, NULL, 0, NULL);
So property_read tries to dereference ppos which is set to NULL when
called from property_write. If property_write is called with NULL args
it may oops too, as it uses args the same way.
I think I can write a patch, just tell me if it has chances to be committed in
the kernel (I'm not a kernel dev). If so, where could I find patch
submission guidelines ?
Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFB2UPUibN+MXHkAogRAqsSAKCKrF5Q/kdgWZrPF++wEnSupzKw7gCfb10s
dWO0Cf6lahwkS6V8p74n3NM=
=EF1b
-----END PGP SIGNATURE-----
--- linux-2.6.10/fs/openpromfs/inode.c 2004-12-24 22:34:44.000000000 +0100
+++ linux-2.6.10-openpromfs/fs/openpromfs/inode.c 2005-01-05 16:35:46.345978153 +0100
@@ -67,14 +67,15 @@
static ssize_t nodenum_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
- struct inode *inode = file->f_dentry->d_inode;
+ struct inode *inode;
char buffer[10];
- if (count < 0 || !inode->u.generic_ip)
+ if (!file || !buf || !ppos || !file->f_dentry || !file->f_dentry->d_inode)
return -EINVAL;
- sprintf (buffer, "%8.8x\n", (u32)(long)(inode->u.generic_ip));
if (file->f_pos >= 9)
return 0;
+ inode = file->f_dentry->d_inode;
+ sprintf (buffer, "%8.8x\n", (u32)(long)(inode->u.generic_ip));
if (count > 9 - file->f_pos)
count = 9 - file->f_pos;
if (copy_to_user(buf, buffer + file->f_pos, count))
@@ -86,7 +87,7 @@
static ssize_t property_read(struct file *filp, char __user *buf,
size_t count, loff_t *ppos)
{
- struct inode *inode = filp->f_dentry->d_inode;
+ struct inode *inode;
int i, j, k;
u32 node;
char *p, *s;
@@ -94,9 +95,12 @@
openprom_property *op;
char buffer[64];
- if (*ppos >= 0xffffff || count >= 0xffffff)
+ if (!filp || count >= 0xffffff || (count && !buf) || (ppos && *ppos >= 0xffffff) || !filp->f_dentry || !filp->f_dentry->d_inode)
return -EINVAL;
+ inode = filp->f_dentry->d_inode;
if (!filp->private_data) {
+ if(!nodes)
+ return -EIO;
node = nodes[(u16)((long)inode->u.generic_ip)].node;
i = ((u32)(long)inode->u.generic_ip) >> 16;
if ((u16)((long)inode->u.generic_ip) == aliases) {
@@ -166,7 +170,7 @@
}
} else
op = (openprom_property *)filp->private_data;
- if (!count || !(op->len || (op->flag & OPP_ASCIIZ)))
+ if (!ppos || !count || !(op->len || (op->flag & OPP_ASCIIZ)))
return 0;
if (op->flag & OPP_STRINGLIST) {
for (k = 0, p = op->value; p < op->value + op->len; p++)
@@ -327,7 +331,7 @@
void *b;
openprom_property *op;
- if (*ppos >= 0xffffff || count >= 0xffffff)
+ if (!filp || !ppos || *ppos >= 0xffffff || count >= 0xffffff || (count && !buf))
return -EINVAL;
if (!filp->private_data) {
i = property_read (filp, NULL, 0, NULL);
@@ -343,13 +347,11 @@
char tmp [9];
int forcelen = 0;
- j = k % 9;
- for (i = 0; i < count; i++, j++) {
- if (j == 9) j = 0;
- if (!j) {
- char ctmp;
- if (get_user(ctmp, &buf[i]))
- return -EFAULT;
+ for (i = 0,j = k; i < count; i++,j++) {
+ char ctmp;
+ if (get_user(ctmp, &buf[i]))
+ return -EFAULT;
+ if (j && !((j+1)%9)) { /* every 9 char exept first of the file */
if (ctmp != '.') {
if (ctmp != '\n') {
if (op->flag & OPP_BINARY)
@@ -363,9 +365,6 @@
}
}
} else {
- char ctmp;
- if (get_user(ctmp, &buf[i]))
- return -EFAULT;
if (ctmp < '0' ||
(ctmp > '9' && ctmp < 'A') ||
(ctmp > 'F' && ctmp < 'a') ||
On Wed, Jan 05, 2005 at 04:39:21PM +0100, Vincent Pelletier wrote:
> Hello.
>
> Here is the patch.
>
> Changelog:
> 2005/01/05 Vincent Pelletier <[email protected]>
> inode.c: (nodenum_read, property_read, property_write):
> Protected against NULL parameters. property_read: Returns 0 when
> called with a 0-length buffer. (property_write): Don't expect an
> hex list to begin with a dot.
NAK. Patch
a) loses needed checks
b) adds utterly bogus ones
c) is much bigger than it should be
All it really takes is
diff -urN RC10-bk6-base/fs/openpromfs/inode.c RC10-bk6-current/fs/openpromfs/inode.c
--- RC10-bk6-base/fs/openpromfs/inode.c 2004-10-18 17:54:07.000000000 -0400
+++ RC10-bk6-current/fs/openpromfs/inode.c 2005-01-05 12:22:08.710924933 -0500
@@ -94,8 +94,6 @@
openprom_property *op;
char buffer[64];
- if (*ppos >= 0xffffff || count >= 0xffffff)
- return -EINVAL;
if (!filp->private_data) {
node = nodes[(u16)((long)inode->u.generic_ip)].node;
i = ((u32)(long)inode->u.generic_ip) >> 16;
@@ -168,6 +166,8 @@
op = (openprom_property *)filp->private_data;
if (!count || !(op->len || (op->flag & OPP_ASCIIZ)))
return 0;
+ if (*ppos >= 0xffffff || count >= 0xffffff)
+ return -EINVAL;
if (op->flag & OPP_STRINGLIST) {
for (k = 0, p = op->value; p < op->value + op->len; p++)
if (!*p)