![]() | |||||||||||||
Messages in this thread |
On Mon, 2002-10-14 at 03:06, Corey Minyard wrote: > Yet one more update, mostly fixes for stylistic things that Randy Dunlap > pointed out, and a bug fix that lets the KCS state machine get out of > the "hosed" state on the next message (buggy hardware can sometimes be > useful :-). As usual, on my home page at http://home.attbi.com/~minyard. > > -Corey +static int ipmi_open(struct inode *inode, struct file *file) .. + priv = kmalloc(sizeof(*priv), GFP_KERNEL); .. + MOD_INC_USE_COUNT; hmm. Ok so you either need the MOD_INC_USE_COUNT or you don't, but if you do it should go before the sleeping kmalloc ;) +static int ipmi_ioctl(struct inode *inode, ... + if (copy_from_user(&req, (void *) data, sizeof(req))) { + rv = -EFAULT; + break; + } + + if (req.addr_len > sizeof(struct ipmi_addr)) + { + rv = -EINVAL; + break; + } + + if (copy_from_user(&addr, req.addr, req.addr_len)) { since addr_len is a signed value, a user that passes -1 will get a LOOOONG copy_from_user scribbling all over kernel memory...same for data_len a few lines onwards +static void sender(void *send_info, .. + if (kcs_info->run_to_completion) { + /* If we are running to completion, then throw it in + the list and run transactions until everything is + clear. Priority doesn't matter here. */ + list_add_tail(&(msg->link), &(kcs_info->xmit_msgs)); + result = kcs_event_handler(kcs_info, 0); + while (result != KCS_SM_IDLE) { + udelay(500); + result = kcs_event_handler(kcs_info, 500); + } is that unconditional udelay with interrupts off really needed? +/* The locking for these for a write lock is done by two locks, first + the "outside" lock then the normal lock. This way, the interfaces + lock can be converted to a read lock without allowing a new write + locker to come in. Note that at interrupt level, this can only be + claimed read, so there is no reason for read lock to save + interrupts. Write locks must still save interrupts because they + can block an interrupt. */ I get the feeling this locking is fishy. A double r/w lock smells bad. really. Either you always take both the same way (eg both for read or both for write) and then the inner lock could be a normal spinlock; or you will deadlock in new and surprising ways.... + spin_lock_irqsave(&interfaces_outside_lock, flags); + write_lock(&interfaces_lock); + for (i=0; i<MAX_IPMI_INTERFACES; i++) { + if (ipmi_interfaces[i] == NULL) { + new_intf = kmalloc(sizeof(*new_intf), GFP_KERNEL); ehm ehm didn't just take TWO spinlocks 3 lines above this sleeping function ? [unhandled content-type:application/pgp-signature] | ||||||||||||
| Last update: 2005-03-22 12:30 [from the cache] ©2003-2008 | |||||||||||||