Hi,
Here are yet more results from the MC project. This checker looks for
inconsistent usage of interrupt functions. For example, it notices when
interrupts can be either on or off when a function exits. It tracks
cli(), sti(), save_flags() and so forth. We've hand-inspected the results
to ensure that the ones you see here are likely to be errors.
As usual, please CC us at [email protected] if you can verify these
potential errors or show that these bugs are false positives.
Important: The code snippets with each bug here were automatically culled
from the source, not manually selected, so they are sometimes inaccurate
as to the actual location of the bug. We've included a comment before
each bug to help in understanding what the checker found, but the only way
to know for sure is to inspect the source.
-Junfeng & Andy
Where the errors are:
+------------------------------------------+----------------------------+
| file | fn |
+------------------------------------------+----------------------------+
| drivers/cdrom/cm206.c | receive_byte |
| drivers/cdrom/cm206.c | send_command |
| drivers/char/ip2/i2lib.c | i2QueueCommands |
| drivers/char/n_r3964.c | add_msg |
| drivers/char/rio/rioroute.c | RIOFixPhbs |
| drivers/char/rio/riotable.c | RIODeleteRta |
| drivers/i2o/i2o_block.c | i2ob_del_device |
| drivers/ide/ide.c | ide_spin_wait_hwgroup |
| drivers/isdn/hisax/config.c | checkcard |
| drivers/isdn/hisax/isar.c | isar_load_firmware |
| drivers/isdn/isdn_ppp.c | isdn_ppp_bind |
| drivers/net/appletalk/cops.c | cops_rx |
| drivers/net/hamradio/soundmodem/sm_wss.c | wss_set_codec_fmt |
| drivers/net/irda/irport.c | irport_net_ioctl |
| drivers/net/irda/irtty.c | irtty_net_ioctl |
| drivers/net/irda/nsc-ircc.c | nsc_ircc_net_ioctl |
| drivers/net/irda/toshoboe.c | toshoboe_net_ioctl |
| drivers/net/irda/w83977af_ir.c | w83977af_net_ioctl |
| drivers/net/pcmcia/wavelan_cs.c | wavelan_get_wireless_stats |
| drivers/net/tokenring/smctr.c | smctr_open_tr |
| drivers/net/wan/comx-hw-mixcom.c | MIXCOM_open |
| drivers/net/wan/lmc/lmc_main.c | lmc_watchdog |
| drivers/scsi/eata_dma.c | eata_queue |
| drivers/scsi/qla1280.c | qla1280_intr_handler |
| drivers/sound/ad1848.c | ad1848_resume |
| drivers/sound/emu10k1/midi.c | emu10k1_midi_callback |
| drivers/sound/sscape.c | sscape_pnp_upload_file |
| net/irda/irttp.c | irttp_proc_read |
+------------------------------------------+----------------------------+
Listing:
[BUG] sleep_or_timeout will call interruptible_sleep_on, which will save disabled flags and then restore them.
/u2/acc/oses/linux/2.4.1/drivers/cdrom/cm206.c:474:send_command: ERROR:INTR:462:474: Interrupts inconsistent, severity `20':474
if (!(inw(r_line_status) & ls_transmitter_buffer_empty)) {
cd->command = command;
Start --->
cli(); /* don't interrupt before sleep */
outw(dc_mask_sync_error | dc_no_stop_on_error |
(inw(r_data_status) & 0x7f), r_data_control);
/* interrupt routine sends command */
Save & Restore
flags here --->
if (sleep_or_timeout(&cd->uart, UART_TIMEOUT)) {
debug(("Time out on write-buffer\n"));
stats(write_timeout);
... DELETED 2 lines ...
}
debug(("Write commmand delayed\n"));
}
else outw(command, r_uart_transmit);
Error --->
}
uch receive_byte(int timeout)
{
uch ret;
---------------------------------------------------------
[BUG] sleep_or_timeout will call interruptible_sleep_on, which will save disabled flags and then restore them.
/u2/acc/oses/linux/2.4.1/drivers/cdrom/cm206.c:499:receive_byte: ERROR:INTR:479:494: Interrupts inconsistent, severity `30':494
{
uch ret;
Start --->
cli();
debug(("cli\n"));
ret = cd->ur[cd->ur_r];
if (cd->ur_r != cd->ur_w) {
sti();
debug(("returning #%d: 0x%x\n", cd->ur_r, cd->ur[cd->ur_r]));
cd->ur_r++; cd->ur_r %= UR_SIZE;
... DELETED 5 lines ...
#ifdef STATISTICS
if (timeout==UART_TIMEOUT) stats(receive_timeout) /* no `;'! */
else stats(dsb_timeout);
#endif
Error --->
return 0xda;
}
ret = cd->ur[cd->ur_r];
debug(("slept; returning #%d: 0x%x\n", cd->ur_r, cd->ur[cd->ur_r]));
cd->ur_r++; cd->ur_r %= UR_SIZE;
---------------------------------------------------------
[BUG] If type != PTYPE_INLINE && type != PTYPE_BYPASS, function will exit with interrupt disabled. need to verify
/u2/acc/oses/linux/2.4.1/drivers/char/ip2/i2lib.c:571:i2QueueCommands: ERROR:INTR:602:753: Interrupts inconsistent, severity `20':753
case PTYPE_INLINE:
lock_var_p = &pCh->Obuf_spinlock;
Start --->
WRITE_LOCK_IRQSAVE(lock_var_p,flags);
stuffIndex = pCh->Obuf_stuff;
bufroom = pCh->Obuf_strip - stuffIndex;
break;
case PTYPE_BYPASS:
lock_var_p = &pCh->Cbuf_spinlock;
WRITE_LOCK_IRQSAVE(lock_var_p,flags);
... DELETED 141 lines ...
// Check for overflow
if (totalsize <= bufroom) {
// Normal Expected path - We still hold LOCK
break; /* from for()- Enough room: goto proceed */
}
......
}
#ifdef IP2DEBUG_TRACE
ip2trace (CHANN, ITRC_QUEUE, ITRC_RETURN, 1, nCommands );
#endif
Error --->
return nCommands; // Good status: number of commands sent
}
//******************************************************************************
// Function: i2GetStatus(pCh,resetBits)
---------------------------------------------------------
[BUG] return with int disabled in an error path
/u2/acc/oses/linux/2.4.1/drivers/char/n_r3964.c:1036:add_msg: ERROR:INTR:990:995: Interrupts inconsistent, severity `20':995
save_flags(flags);
Start --->
cli();
pMsg = kmalloc(sizeof(struct r3964_message), GFP_KERNEL);
TRACE_M("add_msg - kmalloc %x",(int)pMsg);
Return without
enabling interrupt
--->
if(pMsg==NULL)
return;
... DELETED 44 lines ...
if(pClient->sig_flags & R3964_USE_SIGIO)
{
kill_proc(pClient->pid, SIGIO, 1);
}
Error --->
}
static struct r3964_message *remove_msg(struct r3964_info *pInfo,
struct r3964_client_info *pClient)
{
---------------------------------------------------------
[BUG] doesn't enable interrupt in error path
/u2/acc/oses/linux/2.4.1/drivers/char/rio/rioroute.c:713:RIOFixPhbs: ERROR:INTR:653:707: Interrupts inconsistent, severity `20':707
PortP = p->RIOPortp[PortN];
Start --->
rio_spin_lock_irqsave(&PortP->portSem, flags);
/*
** If RTA is not powered on, the tx packets will be
** unset, so go no further.
*/
if (PortP->TxStart == 0) {
rio_dprintk (RIO_DEBUG_ROUTE, "Tx pkts not set up yet\n");
... DELETED 44 lines ...
/*
** Now make sure the range of ports to be serviced includes
** the 2nd 8 on this 16 port RTA.
*/
Error --->
if (link > 3) return;
if (((unit * 8) + 7) > RWORD(HostP->LinkStrP[link].last_port)) {
rio_dprintk (RIO_DEBUG_ROUTE, "last port on host link %d: %d\n", link, (unit * 8) + 7);
WWORD(HostP->LinkStrP[link].last_port, (unit * 8) + 7);
}
---------------------------------------------------------
[BUG] Reuse the same flags variable
/u2/acc/oses/linux/2.4.1/drivers/char/rio/riotable.c:630:RIODeleteRta: ERROR:INTR:510:626: Interrupts inconsistent, severity `20':626
HostP = &p->RIOHosts[host];
Start --->
rio_spin_lock_irqsave( &HostP->HostLock, flags );
if ( (HostP->Flags & RUN_STATE) != RC_RUNNING ) {
use flags
here --->
rio_spin_unlock_irqrestore(&HostP->HostLock, flags);
continue;
}
for ( entry=0; entry<MAX_RUP; entry++ ) {
if ( MapP->RtaUniqueNum == HostP->Mapping[entry].RtaUniqueNum ) {
HostMapP = &HostP->Mapping[entry];
rio_dprintk (RIO_DEBUG_TABLE, "Found entry offset %d on host %s\n",
entry, HostP->Name);
/*
** Check all four links of the unit are disconnected
*/
for ( link=0; link< LINKS_PER_UNIT; link++ ) {
if ( HostMapP->Topology[link].Unit != ROUTE_DISCONNECT ) {
rio_dprintk (RIO_DEBUG_TABLE, "Entry is in use and cannot be deleted!\n");
p->RIOError.Error = UNIT_IS_IN_USE;
rio_spin_unlock_irqrestore( &HostP->HostLock, flags);
return EBUSY;
}
}
/*
** Slot has been allocated, BUT not booted/routed/
** connected/selected or anything else-ed
*/
SysPort = HostMapP->SysPort;
if ( SysPort != NO_PORT ) {
for (port=SysPort; port < SysPort+PORTS_PER_RTA; port++) {
PortP = p->RIOPortp[port];
rio_dprintk (RIO_DEBUG_TABLE, "Unmap port\n");
reuse flags
here --->
rio_spin_lock_irqsave( &PortP->portSem, flags );
... DELETED 106 lines ...
work_done++;
}
}
if ( work_done )
Error --->
return 0;
rio_dprintk (RIO_DEBUG_TABLE, "Couldn't find entry to be deleted\n");
p->RIOError.Error = COULDNT_FIND_ENTRY;
return ENXIO;
---------------------------------------------------------
[BUG] forget to restore flag in error path
/u2/acc/oses/linux/2.4.1/drivers/i2o/i2o_block.c:1426:i2ob_del_device: ERROR:INTR:1416:1496: Interrupts inconsistent, severity `20':1496
int flags;
Start --->
spin_lock_irqsave(&io_request_lock, flags);
/*
* Need to do this...we somtimes get two events from the IRTOS
* in a row and that causes lots of problems.
*/
i2o_device_notify_off(d, &i2o_block_handler);
... DELETED 70 lines ...
i2ob_media_change_flag[unit] = 1;
i2ob_dev_count--;
Error --->
return;
}
/*
* Have we seen a media change ?
---------------------------------------------------------
[BUG] need verify. If hwgroup->busy is not true, the io_request_lock will remain locked
/u2/acc/oses/linux/2.4.1/drivers/ide/ide.c:2335:ide_spin_wait_hwgroup: ERROR:INTR:2320:2335: Interrupts inconsistent, severity `30':2335
unsigned long timeout = jiffies + (3 * HZ);
Start --->
spin_lock_irq(&io_request_lock);
while (hwgroup->busy) {
unsigned long lflags;
spin_unlock_irq(&io_request_lock);
__save_flags(lflags); /* local CPU only */
__sti(); /* local CPU only; needed for jiffies */
... DELETED 5 lines ...
}
__restore_flags(lflags); /* local CPU only */
spin_lock_irq(&io_request_lock);
}
Error --->
return 0;
}
/*
* FIXME: This should be changed to enqueue a special request
---------------------------------------------------------
[BUG] error path. At line 1194, function exits without enabling intr.
/u2/acc/oses/linux/2.4.1/drivers/isdn/hisax/config.c:926:checkcard: ERROR:INTR:917:1177: Interrupts inconsistent, severity `20':1177
save_flags(flags);
Start --->
cli();
if (!(cs = (struct IsdnCardState *)
kmalloc(sizeof(struct IsdnCardState), GFP_ATOMIC))) {
printk(KERN_WARNING
"HiSax: No memory for IsdnCardState(card %d)\n",
cardnr + 1);
restore_flags(flags);
... DELETED 250 lines ...
}
if (!(cs->rcvbuf = kmalloc(MAX_DFRAME_LEN_L1, GFP_ATOMIC))) {
printk(KERN_WARNING
"HiSax: No memory for isac rcvbuf\n");
Error --->
return (1);
}
cs->rcvidx = 0;
cs->tx_skb = NULL;
cs->tx_cnt = 0;
---------------------------------------------------------
[BUG] need verify. In some error paths, they restore flags, but in others, they don't. gotos?
/u2/acc/oses/linux/2.4.1/drivers/isdn/hisax/isar.c:220:isar_load_firmware: ERROR:INTR:348:424: Interrupts inconsistent, severity `10':424
cs->BC_Write_Reg(cs, 0, ISAR_IRQBIT, ISAR_IRQSTA);
save_flags(flags);
Start --->
sti();
cnt = 1000; /* max 1s */
while ((!ireg->bstat) && cnt) {
udelay(1000);
cnt--;
}
if (!cnt) {
... DELETED 66 lines ...
/* disable ISAR IRQ */
cs->BC_Write_Reg(cs, 0, ISAR_IRQBIT, 0);
kfree(msg);
kfree(tmpmsg);
Error --->
return(ret);
}
extern void BChannel_bh(struct BCState *);
#define B_LL_NOCARRIER 8
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/isdn/isdn_ppp.c:190:isdn_ppp_bind: ERROR:INTR:170:205: Interrupts inconsistent, severity `20':205
save_flags(flags);
Start --->
cli();
if (lp->pppbind < 0) { /* device bounded to ippp device ? */
isdn_net_dev *net_dev = dev->netdev;
char exclusive[ISDN_MAX_CHANNELS]; /* exclusive flags */
memset(exclusive, 0, ISDN_MAX_CHANNELS);
while (net_dev) { /* step through net devices to find exclusive minors */
isdn_net_local *lp = net_dev->local;
... DELETED 25 lines ...
}
unit = isdn_ppp_if_get_unit(lp->name); /* get unit number from interface name .. ugly! */
if (unit < 0) {
printk(KERN_ERR "isdn_ppp_bind: illegal interface name %s.\n", lp->name);
Error --->
return -1;
}
lp->ppp_slot = i;
is = ippp_table[i];
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/appletalk/cops.c:776:cops_rx: ERROR:INTR:763:804: Interrupts inconsistent, severity `20':804
save_flags(flags);
Start --->
cli(); /* Disable interrupts. */
if(lp->board==DAYNA)
{
outb(0, ioaddr); /* Send out Zero length. */
outb(0, ioaddr);
outb(DATA_READ, ioaddr); /* Send read command out. */
... DELETED 31 lines ...
dev->name);
lp->stats.rx_dropped++;
while(pkt_len--) /* Discard packet */
inb(ioaddr);
Error --->
return;
}
skb->dev = dev;
skb_put(skb, pkt_len);
skb->protocol = htons(ETH_P_LOCALTALK);
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/hamradio/soundmodem/sm_wss.c:173:wss_set_codec_fmt: ERROR:INTR:164:186: Interrupts inconsistent, severity `20':186
save_flags(flags);
Start --->
cli();
/* Clock and data format register */
write_codec(dev, 0x48, fmt);
if (SCSTATE->crystal) {
write_codec(dev, 0x5c, fmt2 & 0xf0);
/* MCE and interface config reg */
write_codec(dev, 0x49, (fdx ? 0 : 0x4) | (fullcalib ? 0x18 : 0));
... DELETED 12 lines ...
if (!(--time)) {
printk(KERN_WARNING "%s: ad1848 auto calibration timed out (1)\n",
sm_drvname);
restore_flags(flags);
Error --->
return -1;
}
/*
* wait for ACI end
*/
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/irda/irport.c:943:irport_net_ioctl: ERROR:INTR:947:997: Interrupts inconsistent, severity `20':997
/* Disable interrupts & save flags */
save_flags(flags);
Start --->
cli();
switch (cmd) {
case SIOCSBANDWIDTH: /* Set bandwidth */
if (!capable(CAP_NET_ADMIN))
return -EPERM;
irda_task_execute(self, __irport_change_speed, NULL, NULL,
... DELETED 40 lines ...
}
restore_flags(flags);
Error --->
return ret;
}
static struct net_device_stats *irport_net_get_stats(struct net_device *dev)
{
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/irda/irtty.c:963:irtty_net_ioctl: ERROR:INTR:967:1022: Interrupts inconsistent, severity `20':1022
/* Disable interrupts & save flags */
save_flags(flags);
Start --->
cli();
switch (cmd) {
case SIOCSBANDWIDTH: /* Set bandwidth */
if (!capable(CAP_NET_ADMIN))
return -EPERM;
irda_task_execute(self, irtty_change_speed, NULL, NULL,
... DELETED 45 lines ...
}
restore_flags(flags);
Error --->
return ret;
}
static struct net_device_stats *irtty_net_get_stats(struct net_device *dev)
{
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/irda/nsc-ircc.c:1954:nsc_ircc_net_ioctl: ERROR:INTR:1958:1980: Interrupts inconsistent, severity `20':1980
/* Disable interrupts & save flags */
save_flags(flags);
Start --->
cli();
switch (cmd) {
case SIOCSBANDWIDTH: /* Set bandwidth */
if (!capable(CAP_NET_ADMIN))
return -EPERM;
nsc_ircc_change_speed(self, irq->ifr_baudrate);
... DELETED 12 lines ...
}
restore_flags(flags);
Error --->
return ret;
}
static struct net_device_stats *nsc_ircc_net_get_stats(struct net_device *dev)
{
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/irda/toshoboe.c:604:toshoboe_net_ioctl: ERROR:INTR:608:632: Interrupts inconsistent, severity `20':632
/* Disable interrupts & save flags */
save_flags(flags);
Start --->
cli();
switch (cmd) {
case SIOCSBANDWIDTH: /* Set bandwidth */
if (!capable(CAP_NET_ADMIN))
return -EPERM;
/* toshoboe_setbaud(self, irq->ifr_baudrate); */
... DELETED 14 lines ...
}
restore_flags(flags);
Error --->
return ret;
}
#ifdef MODULE
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/irda/w83977af_ir.c:1332:w83977af_net_ioctl: ERROR:INTR:1336:1358: Interrupts inconsistent, severity `20':1358
/* Disable interrupts & save flags */
save_flags(flags);
Start --->
cli();
switch (cmd) {
case SIOCSBANDWIDTH: /* Set bandwidth */
if (!capable(CAP_NET_ADMIN))
return -EPERM;
w83977af_change_speed(self, irq->ifr_baudrate);
... DELETED 12 lines ...
}
restore_flags(flags);
Error --->
return ret;
}
static struct net_device_stats *w83977af_net_get_stats(struct net_device *dev)
{
---------------------------------------------------------
[BUG] error path. this bug is interesting
/u2/acc/oses/linux/2.4.1/drivers/net/pcmcia/wavelan_cs.c:2561:wavelan_get_wireless_stats: ERROR:INTR:2528:2561: Interrupts inconsistent, severity `20':2561
/* Disable interrupts & save flags */
Start --->
spin_lock_irqsave (&lp->lock, flags);
if(lp == (net_local *) NULL)
return (iw_stats *) NULL;
wstats = &lp->wstats;
/* Get data from the mmc */
... DELETED 23 lines ...
#ifdef DEBUG_IOCTL_TRACE
printk(KERN_DEBUG "%s: <-wavelan_get_wireless_stats()\n", dev->name);
#endif
Error --->
return &lp->wstats;
}
#endif /* WIRELESS_EXT */
/************************* PACKET RECEPTION *************************/
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/tokenring/smctr.c:3655:smctr_open_tr: ERROR:INTR:3594:3661: Interrupts inconsistent, severity `20':3661
save_flags(flags);
Start --->
cli();
smctr_set_page(dev, (__u8 *)tp->ram_access);
if((err = smctr_issue_resume_rx_fcb_cmd(dev, (short)MAC_QUEUE)))
return (err);
... DELETED 57 lines ...
}
restore_flags(flags);
Error --->
return (err);
}
/* Check for a network adapter of this type, and return '0 if one exists.
* If dev->base_addr == 0, probe all likely locations.
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/wan/comx-hw-mixcom.c:505:MIXCOM_open: ERROR:INTR:514:562: Interrupts inconsistent, severity `20':562
}
Start --->
save_flags(flags); cli();
if(hw->channel==1) {
request_region(dev->base_addr, MIXCOM_IO_EXTENT, dev->name);
}
if(hw->channel==0 && !(ch->init_status & IRQ_ALLOCATED)) {
... DELETED 38 lines ...
procfile->mode = S_IFREG | 0444;
}
}
Error --->
return 0;
}
static int MIXCOM_close(struct net_device *dev)
{
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/net/wan/lmc/lmc_main.c:728:lmc_watchdog: ERROR:INTR:661:823: Interrupts inconsistent, severity `20':823
lmc_trace(dev, "lmc_watchdog in");
Start --->
spin_lock_irqsave(&sc->lmc_lock, flags);
if(sc->check != 0xBEAFCAFE){
printk("LMC: Corrupt net_device stuct, breaking out\n");
return;
}
... DELETED 152 lines ...
spin_unlock_irqrestore(&sc->lmc_lock, flags);
lmc_trace(dev, "lmc_watchdog out");
Error --->
}
static int lmc_init(struct net_device * const dev) /*fold00*/
{
lmc_trace(dev, "lmc_init in");
---------------------------------------------------------
[BUG] error path.
/u2/acc/oses/linux/2.4.1/drivers/scsi/eata_dma.c:490:eata_queue: ERROR:INTR:464:506: Interrupts inconsistent, severity `20':506
save_flags(flags);
Start --->
cli();
#if 0
for (x = 1, sh = first_HBA; x <= registered_HBAs; x++, sh = SD(sh)->next) {
if(inb((uint)sh->base + HA_RAUXSTAT) & HA_AIRQ) {
printk("eata_dma: scsi%d interrupt pending in eata_queue.\n"
" Calling interrupt handler.\n", sh->host_no);
... DELETED 32 lines ...
printk(KERN_CRIT "eata_queue pid %ld, HBA QUEUE FULL..., "
"returning DID_BUS_BUSY\n", cmd->pid));
done(cmd);
restore_flags(flags);
Error --->
return(0);
}
ccb = &hd->ccb[y];
memset(ccb, 0, sizeof(struct eata_ccb) - sizeof(struct eata_sg_list *));
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/scsi/qla1280.c:1541:qla1280_intr_handler: ERROR:INTR:1522:1594: Interrupts inconsistent, severity `20':1594
}
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,1,95)
Start --->
spin_lock_irqsave(&io_request_lock, cpu_flags);
if(test_and_set_bit(QLA1280_IN_ISR_BIT, &ha->flags))
{
COMTRACE('X')
return;
}
ha->isr_count++;
... DELETED 62 lines ...
WRT_REG_WORD(®->ictrl, ISP_EN_INT + ISP_EN_RISC);
COMTRACE('i')
LEAVE_INTR("qla1280_intr_handler");
Error --->
}
/**************************************************************************
* qla1280_do_dpc
*
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/sound/ad1848.c:2735:ad1848_resume: ERROR:INTR:2732:2769: Interrupts inconsistent, severity `20':2769
save_flags(flags);
Start --->
cli();
/* store old mixer levels */
memcpy(mixer_levels, devc->levels, sizeof (mixer_levels));
ad1848_init_hw(devc);
/* restore mixer levels */
... DELETED 27 lines ...
outb((bits | dma_bits[devc->dma1] | dma2_bit), config_port);
}
restore_flags(flags);
Error --->
return 0;
}
static int ad1848_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
{
---------------------------------------------------------
[BUG] error path( default in a switch)
/u2/acc/oses/linux/2.4.1/drivers/sound/emu10k1/midi.c:426:emu10k1_midi_callback: ERROR:INTR:380:426: Interrupts inconsistent, severity `20':426
DPF(4, "emu10k1_midi_callback()\n");
Start --->
spin_lock_irqsave(&midi_spinlock, flags);
switch (msg) {
case ICARDMIDI_OUTLONGDATA:
midihdr = (struct midi_hdr *) pmsg[2];
kfree(midihdr->data);
... DELETED 36 lines ...
wake_up_interruptible(&midi_dev->iWait);
break;
default: /* Unknown message */
Error --->
return -1;
}
spin_unlock_irqrestore(&midi_spinlock, flags);
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/drivers/sound/sscape.c:1009:sscape_pnp_upload_file: ERROR:INTR:962:1011: Interrupts inconsistent, severity `20':1011
dt = data;
save_flags(flags);
Start --->
cli();
while ( len > 0 ) {
if (len > devc -> buffsize) l = devc->buffsize;
else l = len;
len -= l;
memcpy(devc->raw_buf, dt, l); dt += l;
sscape_start_dma(devc->dma, devc->raw_buf_phys, l, 0x48);
... DELETED 39 lines ...
if ( !done ) printk(KERN_ERR "soundscape: OBP Initialization failed.\n");
sscape_write( devc, 2, devc->ic_type == IC_ODIE ? 0x70 : 0x40);
sscape_write( devc, 3, (devc -> dma << 4) + 0x80);
Error --->
return 1;
}
static void __init sscape_pnp_init_hw(sscape_info* devc)
{
---------------------------------------------------------
[BUG] error path
/u2/acc/oses/linux/2.4.1/net/irda/irttp.c:1595:irttp_proc_read: ERROR:INTR:1547:1595: Interrupts inconsistent, severity `20':1595
save_flags(flags);
Start --->
cli();
self = (struct tsap_cb *) hashbin_get_first(irttp->tsaps);
while (self != NULL) {
if (!self || self->magic != TTP_TSAP_MAGIC)
return len;
... DELETED 38 lines ...
self = (struct tsap_cb *) hashbin_get_next(irttp->tsaps);
}
restore_flags(flags);
Error --->
return len;
}
#endif /* PROC_FS */
Junfeng Yang wrote:
>
> ...
>
> [BUG] sleep_or_timeout will call interruptible_sleep_on, which will save disabled flags and then restore them.
>
> /u2/acc/oses/linux/2.4.1/drivers/cdrom/cm206.c:474:send_command: ERROR:INTR:462:474: Interrupts inconsistent, severity `20':474
>
> if (!(inw(r_line_status) & ls_transmitter_buffer_empty)) {
> cd->command = command;
> Start --->
> cli(); /* don't interrupt before sleep */
> outw(dc_mask_sync_error | dc_no_stop_on_error |
> (inw(r_data_status) & 0x7f), r_data_control);
> /* interrupt routine sends command */
>
> Save & Restore
> flags here --->
> if (sleep_or_timeout(&cd->uart, UART_TIMEOUT)) {
> debug(("Time out on write-buffer\n"));
> stats(write_timeout);
>
> ... DELETED 2 lines ...
>
> }
> debug(("Write commmand delayed\n"));
> }
> else outw(command, r_uart_transmit);
> Error --->
> }
Yes, this is a bug.
> ...
> /u2/acc/oses/linux/2.4.1/drivers/net/irda/irport.c:943:irport_net_ioctl: ERROR:INTR:947:997: Interrupts inconsistent, severity `20':997
>
> /* Disable interrupts & save flags */
> save_flags(flags);
> Start --->
> cli();
>
> switch (cmd) {
> case SIOCSBANDWIDTH: /* Set bandwidth */
> if (!capable(CAP_NET_ADMIN))
> return -EPERM;
> irda_task_execute(self, __irport_change_speed, NULL, NULL,
>
> ... DELETED 40 lines ...
>
> }
>
> restore_flags(flags);
>
> Error --->
> return ret;
> }
Your reporting is a little misleading here.
Yes, there's a bug in this function - the `return -EPERM'
doesn't do a `restore_flags()'. But there is no bug
in the place you've reported.
(Personally, I think *any* C function which has more than
one `return' statement is a bug, and we see a classic
instance here of one of the problems which this practice
can cause. Religious issue. )
> ...
> [BUG] error path
>
> /u2/acc/oses/linux/2.4.1/drivers/net/wan/comx-hw-mixcom.c:505:MIXCOM_open: ERROR:INTR:514:562: Interrupts inconsistent, severity `20':562
>
> }
>
> Start --->
> save_flags(flags); cli();
>
> if(hw->channel==1) {
> request_region(dev->base_addr, MIXCOM_IO_EXTENT, dev->name);
> }
>
> if(hw->channel==0 && !(ch->init_status & IRQ_ALLOCATED)) {
>
> ... DELETED 38 lines ...
>
> procfile->mode = S_IFREG | 0444;
> }
> }
>
> Error --->
> return 0;
> }
There's another problem here. We're calling request_region()
inside cli(). request_region() can sleep.
On SMP, cli() does all sorts of bizarre things which are
quite different between different architectures. I don't
know if this practice is actually unsafe on any architectures,
but it is really bad practice. It's certainly the case that
schedue() will enable interrupts for you, so whatever you're
protecting won't be protected!
So I'd add `sleep inside cli()' to your list of things to
look out for.
Does your tool have the ability to track which functions
can and can't sleep? This is a very common source of bug.
Grab a spinlock, then call a function which calls a function
which calls a function which calls kmalloc(GFP_KERNEL). Unless
the spinlock is always protected by a semaphore, this can deadlock.
>
> /u2/acc/oses/linux/2.4.1/drivers/scsi/eata_dma.c:490:eata_queue: ERROR:INTR:464:506: Interrupts inconsistent, severity `20':506
>
> save_flags(flags);
> Start --->
> cli();
>
> #if 0
> for (x = 1, sh = first_HBA; x <= registered_HBAs; x++, sh = SD(sh)->next) {
> if(inb((uint)sh->base + HA_RAUXSTAT) & HA_AIRQ) {
> printk("eata_dma: scsi%d interrupt pending in eata_queue.\n"
> " Calling interrupt handler.\n", sh->host_no);
>
> ... DELETED 32 lines ...
>
> printk(KERN_CRIT "eata_queue pid %ld, HBA QUEUE FULL..., "
> "returning DID_BUS_BUSY\n", cmd->pid));
> done(cmd);
> restore_flags(flags);
> Error --->
> return(0);
> }
> ccb = &hd->ccb[y];
Something's gone a little wrong here. The bug is in fact about
20 lines higher up.
Generally: yes, everything you've found needs fixing.
> Your reporting is a little misleading here.
Thanks for verifying these bugs ;)
The interrupt checker checks for inconsistent interrupt states. For
example, if a function has one exit point with interrupt disabled, and
another exit point with interrupt enabled, the checker will report an
error at the second exit point. The code snippets are automatically culled
from the source based on the line number in the error report. So the
reporting is sometimes misleading. I'll put the actuall line number in the
comments.
>
> Yes, there's a bug in this function - the `return -EPERM'
> doesn't do a `restore_flags()'. But there is no bug
> in the place you've reported.
>
> (Personally, I think *any* C function which has more than
> one `return' statement is a bug, and we see a classic
> instance here of one of the problems which this practice
> can cause. Religious issue. )
It may be better to have two exit points, one for normal path and one for
error path. All error handling code can be put at the end of the function.
>
>
> > ...
>
> > [BUG] error path
> >
> > /u2/acc/oses/linux/2.4.1/drivers/net/wan/comx-hw-mixcom.c:505:MIXCOM_open: ERROR:INTR:514:562: Interrupts inconsistent, severity `20':562
> >
> > }
> >
> > Start --->
> > save_flags(flags); cli();
> >
> > if(hw->channel==1) {
> > request_region(dev->base_addr, MIXCOM_IO_EXTENT, dev->name);
> > }
> >
> > if(hw->channel==0 && !(ch->init_status & IRQ_ALLOCATED)) {
> >
> > ... DELETED 38 lines ...
> >
> > procfile->mode = S_IFREG | 0444;
> > }
> > }
> >
> > Error --->
> > return 0;
> > }
>
> There's another problem here. We're calling request_region()
> inside cli(). request_region() can sleep.
>
> On SMP, cli() does all sorts of bizarre things which are
> quite different between different architectures. I don't
> know if this practice is actually unsafe on any architectures,
> but it is really bad practice. It's certainly the case that
> schedue() will enable interrupts for you, so whatever you're
> protecting won't be protected!
>
> So I'd add `sleep inside cli()' to your list of things to
> look out for.
>
> Does your tool have the ability to track which functions
> can and can't sleep? This is a very common source of bug.
> Grab a spinlock, then call a function which calls a function
> which calls a function which calls kmalloc(GFP_KERNEL). Unless
> the spinlock is always protected by a semaphore, this can deadlock.
>
> >
> > /u2/acc/oses/linux/2.4.1/drivers/scsi/eata_dma.c:490:eata_queue: ERROR:INTR:464:506: Interrupts inconsistent, severity `20':506
> >
> > save_flags(flags);
> > Start --->
> > cli();
> >
> > #if 0
> > for (x = 1, sh = first_HBA; x <= registered_HBAs; x++, sh = SD(sh)->next) {
> > if(inb((uint)sh->base + HA_RAUXSTAT) & HA_AIRQ) {
> > printk("eata_dma: scsi%d interrupt pending in eata_queue.\n"
> > " Calling interrupt handler.\n", sh->host_no);
> >
> > ... DELETED 32 lines ...
> >
> > printk(KERN_CRIT "eata_queue pid %ld, HBA QUEUE FULL..., "
> > "returning DID_BUS_BUSY\n", cmd->pid));
> > done(cmd);
> > restore_flags(flags);
> > Error --->
> > return(0);
> > }
> > ccb = &hd->ccb[y];
>
> Something's gone a little wrong here. The bug is in fact about
> 20 lines higher up.
>
>
> Generally: yes, everything you've found needs fixing.
>
On Fri, 16 Mar 2001, Junfeng Yang wrote:
> ---------------------------------------------------------
> [BUG] return with int disabled in an error path
>
> /u2/acc/oses/linux/2.4.1/drivers/char/n_r3964.c:1036:add_msg: ERROR:INTR:990:995: Interrupts inconsistent, severity `20':995
>
>
> save_flags(flags);
> Start --->
> cli();
>
> pMsg = kmalloc(sizeof(struct r3964_message), GFP_KERNEL);
> TRACE_M("add_msg - kmalloc %x",(int)pMsg);
> Return without
> enabling interrupt
> --->
> if(pMsg==NULL)
> return;
>
>
> ... DELETED 44 lines ...
>
> if(pClient->sig_flags & R3964_USE_SIGIO)
> {
> kill_proc(pClient->pid, SIGIO, 1);
> }
> Error --->
> }
>
> static struct r3964_message *remove_msg(struct r3964_info *pInfo,
> struct r3964_client_info *pClient)
> {
> ---------------------------------------------------------
The simple 'fix' is to move the offending kmalloc() up above the cli().
That might actually be something else to make an automated test for -
anything which schedules can re-enable interrupts. In general, it's bad to
call anything which can schedule when interrupts are disabled.
But actually, the cli() is there because of the fundamentally flawed
assumption that it provides sufficient locking. I've converted the whole
thing to use spinlocks instead, but haven't got a test setup ATM. I'll
poke at it more on Monday.
akpm, were you looking at this?
--
dwmw2
David Woodhouse wrote:
>
> [ n_r3964.c stuff ]
> ...
> akpm, were you looking at this?
I'm planning on poking through everything which has been
identified as a posible problem. But I won't start for
several weeks - give the maintainers (if any) time to
address these things.
So.. please go ahead :)
There's another thing which needs doing to n_r3964.c, BTW - the
abuse of task queues in r3964_close(). This is, I think, the
only client of task queues which needs to poke so deeply into
the implementation internals and Linus has mentioned something about
needing to redesign the task queues in 2.5. So n_r3964 needs
somehow to be redesigned so that it can use standard APIs.
-
Junfeng Yang wrote:
> [BUG] error path
>
> /u2/acc/oses/linux/2.4.1/drivers/net/appletalk/cops.c:776:cops_rx: ERROR:INTR:763:804: Interrupts inconsistent, severity `20':804
Fixed.
Request: can the checker check for skb's being freed correctly? The
rules:
If an skb is in interrupt context, call dev_kfree_skb_irq.
If an skb might be in interrupt context, call dev_kfree_skb_any.
If an skb is not in interrupt context, call dev_kfree_skb.
I also found and fixed an error of this sort on cops.c as well.
> [BUG] error path. this bug is interesting
>
> /u2/acc/oses/linux/2.4.1/drivers/net/pcmcia/wavelan_cs.c:2561:wavelan_get_wireless_stats: ERROR:INTR:2528:2561: Interrupts inconsistent, severity `20':2561
>
> /* Disable interrupts & save flags */
> Start --->
> spin_lock_irqsave (&lp->lock, flags);
>
> if(lp == (net_local *) NULL)
> return (iw_stats *) NULL;
Fixed.
I dunno WTF the programmer was thinking here... Your de-ref checker
should have caught this too: check 'lp' for NULL after de-referencing
lp->lock.
> [BUG] error path
>
> /u2/acc/oses/linux/2.4.1/drivers/net/tokenring/smctr.c:3655:smctr_open_tr: ERROR:INTR:3594:3661: Interrupts inconsistent, severity `20':3661
Seems to be fixed already.
--
Jeff Garzik | May you have warm words on a cold evening,
Building 1024 | a full mooon on a dark night,
MandrakeSoft | and a smooth road all the way to your door.
On Sun, 18 Mar 2001, Andrew Morton wrote:
> There's another thing which needs doing to n_r3964.c, BTW - the
> abuse of task queues in r3964_close(). This is, I think, the
> only client of task queues which needs to poke so deeply into
> the implementation internals and Linus has mentioned something about
> needing to redesign the task queues in 2.5. So n_r3964 needs
> somehow to be redesigned so that it can use standard APIs.
Hmmm. That particular piece of ugliness seems to have been particularly
gratuitous. We had two task queues, each of which would decrement a
countdown value, calling on_timeout() if it reached zero, and stick the
other on the tq_timer list.
Is there anyone out there that can test this and save me the trouble of
trying to remember how to drive it?
Index: drivers/char/n_r3964.c
===================================================================
RCS file: /inst/cvs/linux/drivers/char/Attic/n_r3964.c,v
retrieving revision 1.1.2.7
diff -u -r1.1.2.7 n_r3964.c
--- drivers/char/n_r3964.c 2001/02/24 19:01:19 1.1.2.7
+++ drivers/char/n_r3964.c 2001/03/18 13:02:49
@@ -13,6 +13,12 @@
* L. Haag
*
* $Log: n_r3964.c,v $
+ * Revision 1.10 2001/03/18 13:02:24 dwmw2
+ * Fix timer usage, use spinlocks properly.
+ *
+ * Revision 1.9 2001/03/18 12:52:14 dwmw2
+ * Merge changes in 2.4.2
+ *
* Revision 1.8 2000/03/23 14:14:54 dwmw2
* Fix race in sleeping in r3964_read()
*
@@ -110,8 +116,6 @@
#define TRACE_Q(fmt, arg...) /**/
#endif
-static void on_timer_1(void*);
-static void on_timer_2(void*);
static void add_tx_queue(struct r3964_info *, struct r3964_block_header *);
static void remove_from_tx_queue(struct r3964_info *pInfo, int error_code);
static void put_char(struct r3964_info *pInfo, unsigned char ch);
@@ -120,7 +124,7 @@
static void transmit_block(struct r3964_info *pInfo);
static void receive_char(struct r3964_info *pInfo, const unsigned char c);
static void receive_error(struct r3964_info *pInfo, const char flag);
-static void on_timeout(struct r3964_info *pInfo);
+static void on_timeout(unsigned long priv);
static int enable_signals(struct r3964_info *pInfo, pid_t pid, int arg);
static int read_telegram(struct r3964_info *pInfo, pid_t pid, unsigned char *buf);
static void add_msg(struct r3964_client_info *pClient, int msg_id, int arg,
@@ -217,7 +221,7 @@
{
int status;
- printk ("r3964: Philips r3964 Driver $Revision: 1.8 $\n");
+ printk ("r3964: Philips r3964 Driver $Revision: 1.10 $\n");
/*
* Register the tty line discipline
@@ -247,40 +251,11 @@
* Protocol implementation routines
*************************************************************/
-static void on_timer_1(void *arg)
-{
- struct r3964_info *pInfo = (struct r3964_info *)arg;
-
- if(pInfo->count_down)
- {
- if(!--pInfo->count_down)
- {
- on_timeout(pInfo);
- }
- }
- queue_task(&pInfo->bh_2, &tq_timer);
-}
-
-static void on_timer_2(void *arg)
-{
- struct r3964_info *pInfo = (struct r3964_info *)arg;
-
- if(pInfo->count_down)
- {
- if(!--pInfo->count_down)
- {
- on_timeout(pInfo);
- }
- }
- queue_task(&pInfo->bh_1, &tq_timer);
-}
-
static void add_tx_queue(struct r3964_info *pInfo, struct r3964_block_header *pHeader)
{
unsigned long flags;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
pHeader->next = NULL;
@@ -294,7 +269,7 @@
pInfo->tx_last = pHeader;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
TRACE_Q("add_tx_queue %x, length %d, tx_first = %x",
(int)pHeader, pHeader->length, (int)pInfo->tx_first );
@@ -337,8 +312,7 @@
wake_up_interruptible (&pInfo->read_wait);
}
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
pInfo->tx_first = pHeader->next;
if(pInfo->tx_first==NULL)
@@ -346,7 +320,7 @@
pInfo->tx_last = NULL;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
kfree(pHeader);
TRACE_M("remove_from_tx_queue - kfree %x",(int)pHeader);
@@ -359,8 +333,7 @@
{
unsigned long flags;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
pHeader->next = NULL;
@@ -375,7 +348,7 @@
}
pInfo->blocks_in_rx_queue++;
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
TRACE_Q("add_rx_queue: %x, length = %d, rx_first = %x, count = %d",
(int)pHeader, pHeader->length,
@@ -396,8 +369,7 @@
TRACE_Q("remove_from_rx_queue: %x, length %d",
(int)pHeader, (int)pHeader->length );
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
if(pInfo->rx_first == pHeader)
{
@@ -430,7 +402,7 @@
}
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
kfree(pHeader);
TRACE_M("remove_from_rx_queue - kfree %x",(int)pHeader);
@@ -471,17 +443,16 @@
unsigned long flags;
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
if((pInfo->state == R3964_IDLE) && (pInfo->tx_first!=NULL))
{
pInfo->state = R3964_TX_REQUEST;
- pInfo->count_down = R3964_TO_QVZ;
pInfo->nRetry=0;
pInfo->flags &= ~R3964_ERROR;
-
- restore_flags(flags);
+ mod_timer(&pInfo->tmr, jiffies + R3964_TO_QVZ);
+
+ spin_unlock_irqrestore(&pInfo->lock, flags);
TRACE_PS("trigger_transmit - sent STX");
@@ -492,7 +463,7 @@
}
else
{
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
}
}
@@ -506,8 +477,8 @@
put_char(pInfo, STX);
flush(pInfo);
pInfo->state = R3964_TX_REQUEST;
- pInfo->count_down = R3964_TO_QVZ;
pInfo->nRetry++;
+ mod_timer(&pInfo->tmr, jiffies + R3964_TO_QVZ);
}
else
{
@@ -566,7 +537,7 @@
put_char(pInfo, pInfo->bcc);
}
pInfo->state = R3964_WAIT_FOR_TX_ACK;
- pInfo->count_down = R3964_TO_QVZ;
+ mod_timer(&pInfo->tmr, jiffies + R3964_TO_QVZ);
}
flush(pInfo);
}
@@ -601,8 +572,8 @@
if(pInfo->nRetry<R3964_MAX_RETRIES)
{
pInfo->state=R3964_WAIT_FOR_RX_REPEAT;
- pInfo->count_down = R3964_TO_RX_PANIC;
pInfo->nRetry++;
+ mod_timer(&pInfo->tmr, jiffies + R3964_TO_RX_PANIC);
}
else
{
@@ -616,7 +587,7 @@
/* received block; submit DLE: */
put_char(pInfo, DLE);
flush(pInfo);
- pInfo->count_down=0;
+ del_timer_sync(&pInfo->tmr);
TRACE_PS(" rx success: got %d chars", length);
/* prepare struct r3964_block_header: */
@@ -701,7 +672,7 @@
TRACE_PE("TRANSMITTING - got illegal char");
pInfo->state = R3964_WAIT_ZVZ_BEFORE_TX_RETRY;
- pInfo->count_down = R3964_TO_ZVZ;
+ mod_timer(&pInfo->tmr, jiffies + R3964_TO_ZVZ);
}
break;
case R3964_WAIT_FOR_TX_ACK:
@@ -728,7 +699,7 @@
{
TRACE_PE("IDLE - got STX but no space in rx_queue!");
pInfo->state=R3964_WAIT_FOR_RX_BUF;
- pInfo->count_down = R3964_TO_NO_BUF;
+ mod_timer(&pInfo->tmr, R3964_TO_NO_BUF);
break;
}
start_receiving:
@@ -738,8 +709,8 @@
pInfo->last_rx = 0;
pInfo->flags &= ~R3964_ERROR;
pInfo->state=R3964_RECEIVING;
- pInfo->count_down = R3964_TO_ZVZ;
- pInfo->nRetry = 0;
+ mod_timer(&pInfo->tmr, R3964_TO_ZVZ);
+ pInfo->nRetry = 0;
put_char(pInfo, DLE);
flush(pInfo);
pInfo->bcc = 0;
@@ -765,7 +736,7 @@
if(pInfo->flags & R3964_BCC)
{
pInfo->state = R3964_WAIT_FOR_BCC;
- pInfo->count_down = R3964_TO_ZVZ;
+ mod_timer(&pInfo->tmr, R3964_TO_ZVZ);
}
else
{
@@ -777,7 +748,7 @@
pInfo->last_rx = c;
char_to_buf:
pInfo->rx_buf[pInfo->rx_position++] = c;
- pInfo->count_down = R3964_TO_ZVZ;
+ mod_timer(&pInfo->tmr, R3964_TO_ZVZ);
}
}
/* else: overflow-msg? BUF_SIZE>MTU; should not happen? */
@@ -818,8 +789,10 @@
}
}
-static void on_timeout(struct r3964_info *pInfo)
+static void on_timeout(unsigned long priv)
{
+ struct r3964_info *pInfo = (void *)priv;
+
switch(pInfo->state)
{
case R3964_TX_REQUEST:
@@ -926,6 +899,7 @@
return -ENOMEM;
TRACE_PS("add client %d to client list", pid);
+ spin_lock_init(&pClient->lock);
pClient->sig_flags=arg;
pClient->pid = pid;
pClient->next=pInfo->firstClient;
@@ -983,14 +957,13 @@
{
queue_the_message:
- save_flags(flags);
- cli();
-
pMsg = kmalloc(sizeof(struct r3964_message), GFP_KERNEL);
TRACE_M("add_msg - kmalloc %x",(int)pMsg);
if(pMsg==NULL)
return;
+ spin_lock_irqsave(&pClient->lock, flags);
+
pMsg->msg_id = msg_id;
pMsg->arg = arg;
pMsg->error_code = error_code;
@@ -1013,7 +986,7 @@
{
pBlock->locks++;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&pClient->lock, flags);
}
else
{
@@ -1048,8 +1021,7 @@
if(pClient->first_msg)
{
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pClient->lock, flags);
pMsg = pClient->first_msg;
pClient->first_msg = pMsg->next;
@@ -1064,7 +1036,7 @@
remove_client_block(pInfo, pClient);
pClient->next_block_to_read = pMsg->block;
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&pClient->lock, flags);
}
return pMsg;
}
@@ -1136,6 +1108,7 @@
return -ENOMEM;
}
+ spin_lock_init(&pInfo->lock);
pInfo->tty = tty;
init_waitqueue_head (&pInfo->read_wait);
pInfo->priority = R3964_MASTER;
@@ -1148,26 +1121,13 @@
pInfo->firstClient=NULL;
pInfo->state=R3964_IDLE;
pInfo->flags = R3964_DEBUG;
- pInfo->count_down = 0;
pInfo->nRetry = 0;
tty->disc_data = pInfo;
-
- /*
- * Add 'on_timer' to timer task queue
- * (will be called from timer bh)
- */
- INIT_LIST_HEAD(&pInfo->bh_1.list);
- pInfo->bh_1.sync = 0;
- pInfo->bh_1.routine = &on_timer_1;
- pInfo->bh_1.data = pInfo;
-
- INIT_LIST_HEAD(&pInfo->bh_2.list);
- pInfo->bh_2.sync = 0;
- pInfo->bh_2.routine = &on_timer_2;
- pInfo->bh_2.data = pInfo;
- queue_task(&pInfo->bh_1, &tq_timer);
+ INIT_LIST_HEAD(&pInfo->tmr.list);
+ pInfo->tmr.data = (unsigned long)pInfo;
+ pInfo->tmr.function = on_timeout;
return 0;
}
@@ -1186,12 +1146,7 @@
* Make sure that our task queue isn't activated. If it
* is, take it out of the linked list.
*/
- spin_lock_irqsave(&tqueue_lock, flags);
- if (pInfo->bh_1.sync)
- list_del(&pInfo->bh_1.list);
- if (pInfo->bh_2.sync)
- list_del(&pInfo->bh_2.list);
- spin_unlock_irqrestore(&tqueue_lock, flags);
+ del_timer_sync(&pInfo->tmr);
/* Remove client-structs and message queues: */
pClient=pInfo->firstClient;
@@ -1212,11 +1167,10 @@
pClient=pNext;
}
/* Remove jobs from tx_queue: */
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
pHeader=pInfo->tx_first;
pInfo->tx_first=pInfo->tx_last=NULL;
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
while(pHeader)
{
@@ -1429,10 +1383,9 @@
if(pClient)
{
poll_wait(file, &pInfo->read_wait, wait);
- save_flags(flags);
- cli();
+ spin_lock_irqsave(&pInfo->lock, flags);
pMsg=pClient->first_msg;
- restore_flags(flags);
+ spin_unlock_irqrestore(&pInfo->lock, flags);
if(pMsg)
result |= POLLIN | POLLRDNORM;
}
Index: include/linux/n_r3964.h
===================================================================
RCS file: /inst/cvs/linux/include/linux/Attic/n_r3964.h,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 n_r3964.h
--- include/linux/n_r3964.h 2000/12/04 15:47:43 1.1.2.1
+++ include/linux/n_r3964.h 2001/03/18 13:02:49
@@ -13,6 +13,12 @@
* L. Haag
*
* $Log: r3964.h,v $
+ * Revision 1.3 2001/03/18 13:02:24 dwmw2
+ * Fix timer usage, use spinlocks properly.
+ *
+ * Revision 1.2 2001/03/18 12:53:15 dwmw2
+ * Merge changes in 2.4.2
+ *
* Revision 1.1.1.1 1998/10/13 16:43:14 dwmw2
* This'll screw the version control
*
@@ -103,8 +109,9 @@
struct r3964_message;
struct r3964_client_info {
+ spinlock_t lock;
pid_t pid;
- unsigned int sig_flags;
+ unsigned int sig_flags;
struct r3964_client_info *next;
@@ -186,6 +193,7 @@
struct r3964_info {
+ spinlock_t lock;
struct tty_struct *tty;
unsigned char priority;
unsigned char *rx_buf; /* ring buffer */
@@ -208,12 +216,9 @@
struct r3964_client_info *firstClient;
unsigned int state;
unsigned int flags;
-
- int count_down;
- int nRetry;
- struct tq_struct bh_1;
- struct tq_struct bh_2;
+ struct timer_list tmr;
+ int nRetry;
};
#endif
--
dwmw2
On Sun, 18 Mar 2001, Andrew Morton wrote:
> I'm planning on poking through everything which has been
> identified as a posible problem. But I won't start for
> several weeks - give the maintainers (if any) time to
> address these things.
I took a look at the ISDN issues, here's a patch which should fix most of
it (sleeping in IRQ, and unchecked user access still missing).
I'ld appreciate it if somebody felt like looking through it. The patch is
against current CVS, but applies against current 2.4.3-pre.
--Kai
diff -ur isdn-HEAD/drivers/isdn/avmb1/capi.c isdn-h/drivers/isdn/avmb1/capi.c
--- isdn-HEAD/drivers/isdn/avmb1/capi.c Thu Mar 15 22:19:21 2001
+++ isdn-h/drivers/isdn/avmb1/capi.c Sat Mar 17 18:21:23 2001
@@ -1082,6 +1082,8 @@
return -ENODEV;
skb = alloc_skb(count, GFP_USER);
+ if (!skb)
+ return -ENOMEM;
if ((retval = copy_from_user(skb_put(skb, count), buf, count))) {
kfree_skb(skb);
@@ -1501,6 +1503,8 @@
return -EINVAL;
skb = alloc_skb(CAPI_DATA_B3_REQ_LEN+count, GFP_USER);
+ if (!skb)
+ return -ENOMEM;
skb_reserve(skb, CAPI_DATA_B3_REQ_LEN);
if ((retval = copy_from_user(skb_put(skb, count), buf, count))) {
Only in isdn-h/drivers/isdn/avmb1: capi.c%
diff -ur isdn-HEAD/drivers/isdn/avmb1/capidrv.c isdn-h/drivers/isdn/avmb1/capidrv.c
--- isdn-HEAD/drivers/isdn/avmb1/capidrv.c Thu Mar 15 17:05:56 2001
+++ isdn-h/drivers/isdn/avmb1/capidrv.c Sat Mar 17 18:23:33 2001
@@ -2065,8 +2065,8 @@
__u16 datahandle;
if (!card) {
- printk(KERN_ERR "capidrv-%d: if_sendbuf called with invalid driverId %d!\n",
- card->contrnr, id);
+ printk(KERN_ERR "capidrv: if_sendbuf called with invalid driverId %d!\n",
+ id);
return 0;
}
if (debugmode > 1)
@@ -2137,8 +2137,8 @@
__u8 *p;
if (!card) {
- printk(KERN_ERR "capidrv-%d: if_readstat called with invalid driverId %d!\n",
- card->contrnr, id);
+ printk(KERN_ERR "capidrv: if_readstat called with invalid driverId %d!\n",
+ id);
return -ENODEV;
}
Only in isdn-h/drivers/isdn/avmb1: capidrv.c%
diff -ur isdn-HEAD/drivers/isdn/hisax/config.c isdn-h/drivers/isdn/hisax/config.c
--- isdn-HEAD/drivers/isdn/hisax/config.c Thu Mar 15 22:19:21 2001
+++ isdn-h/drivers/isdn/hisax/config.c Sat Mar 17 18:07:41 2001
@@ -925,13 +925,12 @@
save_flags(flags);
cli();
- if (!(cs = (struct IsdnCardState *)
- kmalloc(sizeof(struct IsdnCardState), GFP_ATOMIC))) {
+ cs = kmalloc(sizeof(struct IsdnCardState), GFP_ATOMIC)
+ if (!cs) {
printk(KERN_WARNING
"HiSax: No memory for IsdnCardState(card %d)\n",
cardnr + 1);
- restore_flags(flags);
- return (0);
+ goto out;
}
memset(cs, 0, sizeof(struct IsdnCardState));
card->cs = cs;
@@ -950,241 +949,235 @@
#endif
cs->protocol = card->protocol;
- if ((card->typ > 0) && (card->typ <= ISDN_CTYPE_COUNT)) {
- if (!(cs->dlog = kmalloc(MAX_DLOG_SPACE, GFP_ATOMIC))) {
- printk(KERN_WARNING
- "HiSax: No memory for dlog(card %d)\n",
- cardnr + 1);
- restore_flags(flags);
- return (0);
- }
- if (!(cs->status_buf = kmalloc(HISAX_STATUS_BUFSIZE, GFP_ATOMIC))) {
- printk(KERN_WARNING
- "HiSax: No memory for status_buf(card %d)\n",
- cardnr + 1);
- kfree(cs->dlog);
- restore_flags(flags);
- return (0);
- }
- cs->stlist = NULL;
- cs->status_read = cs->status_buf;
- cs->status_write = cs->status_buf;
- cs->status_end = cs->status_buf + HISAX_STATUS_BUFSIZE - 1;
- cs->typ = card->typ;
- strcpy(cs->iif.id, id);
- cs->iif.channels = 2;
- cs->iif.maxbufsize = MAX_DATA_SIZE;
- cs->iif.hl_hdrlen = MAX_HEADER_LEN;
- cs->iif.features =
- ISDN_FEATURE_L2_X75I |
- ISDN_FEATURE_L2_HDLC |
- ISDN_FEATURE_L2_HDLC_56K |
- ISDN_FEATURE_L2_TRANS |
- ISDN_FEATURE_L3_TRANS |
+ if (card->typ <= 0 || card->typ > ISDN_CTYPE_COUNT) {
+ printk(KERN_WARNING
+ "HiSax: Card Type %d out of range\n",
+ card->typ);
+ goto outf_cs;
+ }
+ if (!(cs->dlog = kmalloc(MAX_DLOG_SPACE, GFP_ATOMIC))) {
+ printk(KERN_WARNING
+ "HiSax: No memory for dlog(card %d)\n",
+ cardnr + 1);
+ goto outf_cs;
+ }
+ if (!(cs->status_buf = kmalloc(HISAX_STATUS_BUFSIZE, GFP_ATOMIC))) {
+ printk(KERN_WARNING
+ "HiSax: No memory for status_buf(card %d)\n",
+ cardnr + 1);
+ goto outf_dlog;
+ }
+ cs->stlist = NULL;
+ cs->status_read = cs->status_buf;
+ cs->status_write = cs->status_buf;
+ cs->status_end = cs->status_buf + HISAX_STATUS_BUFSIZE - 1;
+ cs->typ = card->typ;
+ strcpy(cs->iif.id, id);
+ cs->iif.channels = 2;
+ cs->iif.maxbufsize = MAX_DATA_SIZE;
+ cs->iif.hl_hdrlen = MAX_HEADER_LEN;
+ cs->iif.features =
+ ISDN_FEATURE_L2_X75I |
+ ISDN_FEATURE_L2_HDLC |
+ ISDN_FEATURE_L2_HDLC_56K |
+ ISDN_FEATURE_L2_TRANS |
+ ISDN_FEATURE_L3_TRANS |
#ifdef CONFIG_HISAX_1TR6
- ISDN_FEATURE_P_1TR6 |
+ ISDN_FEATURE_P_1TR6 |
#endif
#ifdef CONFIG_HISAX_EURO
- ISDN_FEATURE_P_EURO |
+ ISDN_FEATURE_P_EURO |
#endif
#ifdef CONFIG_HISAX_NI1
- ISDN_FEATURE_P_NI1 |
+ ISDN_FEATURE_P_NI1 |
#endif
- 0;
-
- cs->iif.command = HiSax_command;
- cs->iif.writecmd = NULL;
- cs->iif.writebuf_skb = HiSax_writebuf_skb;
- cs->iif.readstat = HiSax_readstatus;
- register_isdn(&cs->iif);
- cs->myid = cs->iif.channels;
- printk(KERN_INFO
- "HiSax: Card %d Protocol %s Id=%s (%d)\n", cardnr + 1,
- (card->protocol == ISDN_PTYPE_1TR6) ? "1TR6" :
- (card->protocol == ISDN_PTYPE_EURO) ? "EDSS1" :
- (card->protocol == ISDN_PTYPE_LEASED) ? "LEASED" :
- (card->protocol == ISDN_PTYPE_NI1) ? "NI1" :
- "NONE", cs->iif.id, cs->myid);
- switch (card->typ) {
+ 0;
+
+ cs->iif.command = HiSax_command;
+ cs->iif.writecmd = NULL;
+ cs->iif.writebuf_skb = HiSax_writebuf_skb;
+ cs->iif.readstat = HiSax_readstatus;
+ register_isdn(&cs->iif);
+ cs->myid = cs->iif.channels;
+ printk(KERN_INFO
+ "HiSax: Card %d Protocol %s Id=%s (%d)\n", cardnr + 1,
+ (card->protocol == ISDN_PTYPE_1TR6) ? "1TR6" :
+ (card->protocol == ISDN_PTYPE_EURO) ? "EDSS1" :
+ (card->protocol == ISDN_PTYPE_LEASED) ? "LEASED" :
+ (card->protocol == ISDN_PTYPE_NI1) ? "NI1" :
+ "NONE", cs->iif.id, cs->myid);
+ switch (card->typ) {
#if CARD_TELES0
- case ISDN_CTYPE_16_0:
- case ISDN_CTYPE_8_0:
- ret = setup_teles0(card);
- break;
+ case ISDN_CTYPE_16_0:
+ case ISDN_CTYPE_8_0:
+ ret = setup_teles0(card);
+ break;
#endif
#if CARD_TELES3
- case ISDN_CTYPE_16_3:
- case ISDN_CTYPE_PNP:
- case ISDN_CTYPE_TELESPCMCIA:
- case ISDN_CTYPE_COMPAQ_ISA:
- ret = setup_teles3(card);
- break;
+ case ISDN_CTYPE_16_3:
+ case ISDN_CTYPE_PNP:
+ case ISDN_CTYPE_TELESPCMCIA:
+ case ISDN_CTYPE_COMPAQ_ISA:
+ ret = setup_teles3(card);
+ break;
#endif
#if CARD_S0BOX
- case ISDN_CTYPE_S0BOX:
- ret = setup_s0box(card);
- break;
+ case ISDN_CTYPE_S0BOX:
+ ret = setup_s0box(card);
+ break;
#endif
#if CARD_TELESPCI
- case ISDN_CTYPE_TELESPCI:
- ret = setup_telespci(card);
- break;
+ case ISDN_CTYPE_TELESPCI:
+ ret = setup_telespci(card);
+ break;
#endif
#if CARD_AVM_A1
- case ISDN_CTYPE_A1:
- ret = setup_avm_a1(card);
- break;
+ case ISDN_CTYPE_A1:
+ ret = setup_avm_a1(card);
+ break;
#endif
#if CARD_AVM_A1_PCMCIA
- case ISDN_CTYPE_A1_PCMCIA:
- ret = setup_avm_a1_pcmcia(card);
- break;
+ case ISDN_CTYPE_A1_PCMCIA:
+ ret = setup_avm_a1_pcmcia(card);
+ break;
#endif
#if CARD_FRITZPCI
- case ISDN_CTYPE_FRITZPCI:
- ret = setup_avm_pcipnp(card);
- break;
+ case ISDN_CTYPE_FRITZPCI:
+ ret = setup_avm_pcipnp(card);
+ break;
#endif
#if CARD_ELSA
- case ISDN_CTYPE_ELSA:
- case ISDN_CTYPE_ELSA_PNP:
- case ISDN_CTYPE_ELSA_PCMCIA:
- case ISDN_CTYPE_ELSA_PCI:
- ret = setup_elsa(card);
- break;
+ case ISDN_CTYPE_ELSA:
+ case ISDN_CTYPE_ELSA_PNP:
+ case ISDN_CTYPE_ELSA_PCMCIA:
+ case ISDN_CTYPE_ELSA_PCI:
+ ret = setup_elsa(card);
+ break;
#endif
#if CARD_IX1MICROR2
- case ISDN_CTYPE_IX1MICROR2:
- ret = setup_ix1micro(card);
- break;
+ case ISDN_CTYPE_IX1MICROR2:
+ ret = setup_ix1micro(card);
+ break;
#endif
#if CARD_DIEHLDIVA
- case ISDN_CTYPE_DIEHLDIVA:
- ret = setup_diva(card);
- break;
+ case ISDN_CTYPE_DIEHLDIVA:
+ ret = setup_diva(card);
+ break;
#endif
#if CARD_ASUSCOM
- case ISDN_CTYPE_ASUSCOM:
- ret = setup_asuscom(card);
- break;
+ case ISDN_CTYPE_ASUSCOM:
+ ret = setup_asuscom(card);
+ break;
#endif
#if CARD_TELEINT
- case ISDN_CTYPE_TELEINT:
- ret = setup_TeleInt(card);
- break;
+ case ISDN_CTYPE_TELEINT:
+ ret = setup_TeleInt(card);
+ break;
#endif
#if CARD_SEDLBAUER
- case ISDN_CTYPE_SEDLBAUER:
- case ISDN_CTYPE_SEDLBAUER_PCMCIA:
- case ISDN_CTYPE_SEDLBAUER_FAX:
- ret = setup_sedlbauer(card);
- break;
+ case ISDN_CTYPE_SEDLBAUER:
+ case ISDN_CTYPE_SEDLBAUER_PCMCIA:
+ case ISDN_CTYPE_SEDLBAUER_FAX:
+ ret = setup_sedlbauer(card);
+ break;
#endif
#if CARD_SPORTSTER
- case ISDN_CTYPE_SPORTSTER:
- ret = setup_sportster(card);
- break;
+ case ISDN_CTYPE_SPORTSTER:
+ ret = setup_sportster(card);
+ break;
#endif
#if CARD_MIC
- case ISDN_CTYPE_MIC:
- ret = setup_mic(card);
- break;
+ case ISDN_CTYPE_MIC:
+ ret = setup_mic(card);
+ break;
#endif
#if CARD_NETJET_S
- case ISDN_CTYPE_NETJET_S:
- ret = setup_netjet_s(card);
- break;
+ case ISDN_CTYPE_NETJET_S:
+ ret = setup_netjet_s(card);
+ break;
#endif
#if CARD_HFCS
- case ISDN_CTYPE_TELES3C:
- case ISDN_CTYPE_ACERP10:
- ret = setup_hfcs(card);
- break;
+ case ISDN_CTYPE_TELES3C:
+ case ISDN_CTYPE_ACERP10:
+ ret = setup_hfcs(card);
+ break;
#endif
#if CARD_HFC_PCI
- case ISDN_CTYPE_HFC_PCI:
- ret = setup_hfcpci(card);
- break;
+ case ISDN_CTYPE_HFC_PCI:
+ ret = setup_hfcpci(card);
+ break;
#endif
#if CARD_HFC_SX
- case ISDN_CTYPE_HFC_SX:
- ret = setup_hfcsx(card);
- break;
+ case ISDN_CTYPE_HFC_SX:
+ ret = setup_hfcsx(card);
+ break;
#endif
#if CARD_NICCY
- case ISDN_CTYPE_NICCY:
- ret = setup_niccy(card);
- break;
+ case ISDN_CTYPE_NICCY:
+ ret = setup_niccy(card);
+ break;
#endif
#if CARD_AMD7930
- case ISDN_CTYPE_AMD7930:
- ret = setup_amd7930(card);
- break;
+ case ISDN_CTYPE_AMD7930:
+ ret = setup_amd7930(card);
+ break;
#endif
#if CARD_ISURF
- case ISDN_CTYPE_ISURF:
- ret = setup_isurf(card);
- break;
+ case ISDN_CTYPE_ISURF:
+ ret = setup_isurf(card);
+ break;
#endif
#if CARD_HSTSAPHIR
- case ISDN_CTYPE_HSTSAPHIR:
- ret = setup_saphir(card);
- break;
+ case ISDN_CTYPE_HSTSAPHIR:
+ ret = setup_saphir(card);
+ break;
#endif
#if CARD_TESTEMU
- case ISDN_CTYPE_TESTEMU:
- ret = setup_testemu(card);
- break;
+ case ISDN_CTYPE_TESTEMU:
+ ret = setup_testemu(card);
+ break;
#endif
#if CARD_BKM_A4T
- case ISDN_CTYPE_BKM_A4T:
- ret = setup_bkm_a4t(card);
- break;
+ case ISDN_CTYPE_BKM_A4T:
+ ret = setup_bkm_a4t(card);
+ break;
#endif
#if CARD_SCT_QUADRO
- case ISDN_CTYPE_SCT_QUADRO:
- ret = setup_sct_quadro(card);
- break;
+ case ISDN_CTYPE_SCT_QUADRO:
+ ret = setup_sct_quadro(card);
+ break;
#endif
#if CARD_GAZEL
- case ISDN_CTYPE_GAZEL:
- ret = setup_gazel(card);
- break;
+ case ISDN_CTYPE_GAZEL:
+ ret = setup_gazel(card);
+ break;
#endif
#if CARD_W6692
- case ISDN_CTYPE_W6692:
- ret = setup_w6692(card);
- break;
+ case ISDN_CTYPE_W6692:
+ ret = setup_w6692(card);
+ break;
#endif
#if CARD_NETJET_U
- case ISDN_CTYPE_NETJET_U:
- ret = setup_netjet_u(card);
- break;
-#endif
- default:
- printk(KERN_WARNING
- "HiSax: Support for %s Card not selected\n",
- CardType[card->typ]);
- ll_unload(cs);
- restore_flags(flags);
- return (0);
- }
- } else {
+ case ISDN_CTYPE_NETJET_U:
+ ret = setup_netjet_u(card);
+ break;
+#endif
+ default:
printk(KERN_WARNING
- "HiSax: Card Type %d out of range\n",
- card->typ);
- restore_flags(flags);
- return (0);
+ "HiSax: Support for %s Card not selected\n",
+ CardType[card->typ]);
+ ll_unload(cs);
+ goto outf_cs;
}
if (!ret) {
ll_unload(cs);
- restore_flags(flags);
- return (0);
+ goto outf_cs;
}
if (!(cs->rcvbuf = kmalloc(MAX_DFRAME_LEN_L1, GFP_ATOMIC))) {
printk(KERN_WARNING
"HiSax: No memory for isac rcvbuf\n");
- return (1);
+ ll_unload(cs);
+ goto outf_cs;
}
cs->rcvidx = 0;
cs->tx_skb = NULL;
@@ -1201,21 +1194,31 @@
ret = init_card(cs);
if (ret) {
closecard(cardnr);
- restore_flags(flags);
- return (0);
+ ret = 0;
+ goto outf_cs;
}
init_tei(cs, cs->protocol);
ret = CallcNewChan(cs);
if (ret) {
closecard(cardnr);
- restore_flags(flags);
- return 0;
+ ret = 0;
+ goto outf_cs;
}
/* ISAR needs firmware download first */
if (!test_bit(HW_ISAR, &cs->HW_Flags))
ll_run(cs, 0);
+
+ ret = 1;
+ goto out;
+
+ outf_dlog:
+ kfree(cs->dlog);
+ outf_cs:
+ kfree(cs);
+ card->cs = NULL;
+ out:
restore_flags(flags);
- return (1);
+ return ret;
}
void __devinit
@@ -1264,9 +1267,6 @@
} else {
printk(KERN_WARNING "HiSax: Card %s not installed !\n",
CardType[cards[i].typ]);
- if (cards[i].cs)
- kfree((void *) cards[i].cs);
- cards[i].cs = NULL;
HiSax_shiftcards(i);
nrcards--;
}
Only in isdn-h/drivers/isdn/hisax: config.c%
diff -ur isdn-HEAD/drivers/isdn/hisax/isar.c isdn-h/drivers/isdn/hisax/isar.c
--- isdn-HEAD/drivers/isdn/hisax/isar.c Thu Mar 15 10:08:37 2001
+++ isdn-h/drivers/isdn/hisax/isar.c Sat Mar 17 18:10:54 2001
@@ -383,12 +383,12 @@
} else {
printk(KERN_DEBUG"isar selftest not OK %x/%x/%x\n",
ireg->cmsb, ireg->clsb, ireg->par[0]);
- ret = 1;goto reterror;
+ ret = 1;goto reterrflg;
}
ireg->iis = 0;
if (!sendmsg(cs, ISAR_HIS_DIAG, ISAR_CTRL_SWVER, 0, NULL)) {
printk(KERN_ERR"isar RQST SVN failed\n");
- ret = 1;goto reterror;
+ ret = 1;goto reterrflg;
}
cnt = 30000; /* max 300 ms */
while ((ireg->iis != ISAR_IIS_DIAG) && cnt) {
Only in isdn-h/drivers/isdn/hisax: isar.c%
diff -ur isdn-HEAD/drivers/isdn/isdn_ppp.c isdn-h/drivers/isdn/isdn_ppp.c
--- isdn-HEAD/drivers/isdn/isdn_ppp.c Wed Jan 24 00:22:46 2001
+++ isdn-h/drivers/isdn/isdn_ppp.c Sat Mar 17 18:13:59 2001
@@ -176,6 +176,7 @@
int unit = 0;
long flags;
struct ippp_struct *is;
+ int retval;
save_flags(flags);
cli();
@@ -208,12 +209,14 @@
if (i >= ISDN_MAX_CHANNELS) {
restore_flags(flags);
printk(KERN_WARNING "isdn_ppp_bind: Can't find a (free) connection to the ipppd daemon.\n");
- return -1;
+ retval = -1;
+ goto out;
}
unit = isdn_ppp_if_get_unit(lp->name); /* get unit number from interface name .. ugly! */
if (unit < 0) {
printk(KERN_ERR "isdn_ppp_bind: illegal interface name %s.\n", lp->name);
- return -1;
+ retval = -1;
+ goto out;
}
lp->ppp_slot = i;
@@ -222,13 +225,16 @@
is->unit = unit;
is->state = IPPP_OPEN | IPPP_ASSIGNED; /* assigned to a netdevice but not connected */
#ifdef CONFIG_ISDN_MPP
- if (isdn_ppp_mp_init(lp, NULL) < 0)
- return -ENOMEM;
+ retval = isdn_ppp_mp_init(lp, NULL);
+ if (retval < 0)
+ goto out;
#endif /* CONFIG_ISDN_MPP */
- restore_flags(flags);
+ retval = lp->ppp_slot;
- return lp->ppp_slot;
+ out:
+ restore_flags(flags);
+ return retval;
}
/*
Only in isdn-h/drivers/isdn: isdn_ppp.c%
diff -ur isdn-HEAD/drivers/isdn/pcbit/drv.c isdn-h/drivers/isdn/pcbit/drv.c
--- isdn-HEAD/drivers/isdn/pcbit/drv.c Thu Mar 15 10:08:38 2001
+++ isdn-h/drivers/isdn/pcbit/drv.c Sat Mar 17 18:30:32 2001
@@ -430,7 +430,6 @@
return len;
}
-
int pcbit_writecmd(const u_char* buf, int len, int user, int driver, int channel)
{
struct pcbit_dev * dev;
@@ -454,32 +453,36 @@
if (len > BANK4 + 1)
{
printk("pcbit_writecmd: invalid length %d\n", len);
- return -EFAULT;
+ return -EINVAL;
}
if (user)
{
- u_char cbuf[1024];
+ u_char *cbuf = kmalloc(len, GFP_KERNEL);
+ if (!cbuf)
+ return -ENOMEM;
- copy_from_user(cbuf, buf, len);
- for (i=0; i<len; i++)
- writeb(cbuf[i], dev->sh_mem + i);
+ if (copy_from_user(cbuf, buf, len)) {
+ kfree(cbuf);
+ return -EFAULT;
+ }
+ memcpy_toio(dev->sh_mem, cbuf, len);
+ kfree(cbuf);
}
else
memcpy_toio(dev->sh_mem, buf, len);
return len;
- break;
case L2_FWMODE:
/* this is the hard part */
/* dumb board */
- if (len < 0)
- return -EINVAL;
-
if (user) {
/* get it into kernel space */
if ((ptr = kmalloc(len, GFP_KERNEL))==NULL)
return -ENOMEM;
- copy_from_user(ptr, buf, len);
+ if (copy_from_user(ptr, buf, len)) {
+ kfree(ptr);
+ return -EFAULT;
+ }
loadbuf = ptr;
}
else
@@ -511,12 +514,9 @@
kfree(ptr);
return errstat ? errstat : len;
-
- break;
default:
return -EBUSY;
}
- return 0;
}
/*
Only in isdn-h/drivers/isdn/pcbit: drv.c%
diff -ur isdn-HEAD/drivers/isdn/sc/interrupt.c isdn-h/drivers/isdn/sc/interrupt.c
--- isdn-HEAD/drivers/isdn/sc/interrupt.c Thu Mar 15 10:08:38 2001
+++ isdn-h/drivers/isdn/sc/interrupt.c Sat Mar 17 17:35:12 2001
@@ -34,7 +34,6 @@
extern int indicate_status(int, int, ulong, char *);
extern void check_phystat(unsigned long);
-extern void dump_messages(int);
extern int receivemessage(int, RspMessage *);
extern int sendmessage(int, unsigned int, unsigned int, unsigned int,
unsigned int, unsigned int, unsigned int, unsigned int *);
Only in isdn-h/drivers/isdn/sc: interrupt.c%
diff -ur isdn-HEAD/drivers/isdn/sc/message.c isdn-h/drivers/isdn/sc/message.c
--- isdn-HEAD/drivers/isdn/sc/message.c Sat Sep 4 08:20:07 1999
+++ isdn-h/drivers/isdn/sc/message.c Sat Mar 17 17:35:43 2001
@@ -38,55 +38,12 @@
extern unsigned int cinst;
/*
- * Obligitory function prototypes
+ * Obligatory function prototypes
*/
extern int indicate_status(int,ulong,char*);
extern int scm_command(isdn_ctrl *);
extern void *memcpy_fromshmem(int, void *, const void *, size_t);
-/*
- * Dump message queue in shared memory to screen
- */
-void dump_messages(int card)
-{
- DualPortMemory dpm;
- unsigned long flags;
-
- int i =0;
-
- if (!IS_VALID_CARD(card)) {
- pr_debug("Invalid param: %d is not a valid card id\n", card);
- }
-
- save_flags(flags);
- cli();
- outb(adapter[card]->ioport[adapter[card]->shmem_pgport],
- (adapter[card]->shmem_magic >> 14) | 0x80);
- memcpy_fromshmem(card, &dpm, 0, sizeof(dpm));
- restore_flags(flags);
-
- pr_debug("%s: Dumping Request Queue\n", adapter[card]->devicename);
- for (i = 0; i < dpm.req_head; i++) {
- pr_debug("%s: Message #%d: (%d,%d,%d), link: %d\n",
- adapter[card]->devicename, i,
- dpm.req_queue[i].type,
- dpm.req_queue[i].class,
- dpm.req_queue[i].code,
- dpm.req_queue[i].phy_link_no);
- }
-
- pr_debug("%s: Dumping Response Queue\n", adapter[card]->devicename);
- for (i = 0; i < dpm.rsp_head; i++) {
- pr_debug("%s: Message #%d: (%d,%d,%d), link: %d, status: %d\n",
- adapter[card]->devicename, i,
- dpm.rsp_queue[i].type,
- dpm.rsp_queue[i].class,
- dpm.rsp_queue[i].code,
- dpm.rsp_queue[i].phy_link_no,
- dpm.rsp_queue[i].rsp_status);
- }
-
-}
/*
* receive a message from the board
Only in isdn-h/drivers/isdn/sc: message.c%
> Request: can the checker check for skb's being freed correctly? The
> rules:
>
> If an skb is in interrupt context, call dev_kfree_skb_irq.
> If an skb might be in interrupt context, call dev_kfree_skb_any.
> If an skb is not in interrupt context, call dev_kfree_skb.
It shouldn't be hard to check this. The only thing interesting will be
deriving when you're in an interrupt context. Thanks for the pointer.
Are there other context-sensitive rules that we could go after as well?
> I dunno WTF the programmer was thinking here... Your de-ref checker
> should have caught this too: check 'lp' for NULL after de-referencing
> lp->lock.
These reports are for an older version of the checker --- we've fixed
some bugs in the system, which should catch more of these errors.
Dawson