Re: locking against myself

2004-02-19 Thread Andrey Simonenko
On Wed, Feb 18, 2004 at 11:52:26AM -0800, Jerry Toung wrote:
> Hello hackers,
> I am constantly getting the following message when I run my KLD:
> 
> panic: lockmgr: locking against myself
> Debugger("panic")
> Stopped atDebugger+0x54:  xchgl   %ebx,in_Debugger.0
> db>
> 
> What could possibly cause this?
> It seem to say that I'm locking an already locked variable.
> 

Hello Jerry,

According to kern/kern_lock.c:lockmgr() your proc/thread already
has an exclusive lock and it tries to obtain a recursive exclusive
lock and lock doesn't have LK_CANRECURSE flag (also you allow to
sleep in lockmng()).

This is documented in lock(9) manual page:

   LK_EXCLUSIVEAcquire an exclusive lock.  If an exclusive
   lock is already held, and LK_CANRECURSE is not
   set, the system will panic(9).
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: FreeBSD 5.2 v/s FreeBSD 4.9 MFLOPS performance (gcc3.3.3 v/s gcc2.9.5)

2004-02-19 Thread Wes Peters
On Monday 16 February 2004 10:11 am, Dag-Erling Smørgrav wrote:
> Kris Kennaway <[EMAIL PROTECTED]> writes:
> > On Mon, Feb 16, 2004 at 03:52:16AM -0800, Wes Peters wrote:
> > > Should I commit this?
> >
> > What effect does it have on non-i386 architectures?
>
> It can't possibly hurt.  If the stack is already aligned on a "better"
> boundary (64 or 128 bytes), it is also aligned on a 32-byte boundary
> since 64 and 128 are multiples of 32, and the patch is a no-op.  If
> only a 16-byte alignment is required, a 32-byte alignment wastes a
> small amount of memory but does not hurt performance.  I believe that
> less-than-16 (and possibly even less-than-32) alignment is pessimal on
> all platforms we support.

I'm building world on my sparc64 just to be sure.  Sorry, I didn't get to 
work on this at all last night, but I should be able to post conclusive 
results tonight, I just have to get through the rather long buildworld 
while I'm at the office today.

Thank ${DEITY} for cheap, fast AMD machines. ;^)

-- 

Where am I, and what am I doing in this handbasket?

Wes Peters   [EMAIL PROTECTED]
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: locking against myself

2004-02-19 Thread Jerry Toung
Thanks a lot Andrey.

On Thursday 19 February 2004 12:05 am, Andrey Simonenko wrote:
> On Wed, Feb 18, 2004 at 11:52:26AM -0800, Jerry Toung wrote:
> > Hello hackers,
> > I am constantly getting the following message when I run my KLD:
> >
> > panic: lockmgr: locking against myself
> > Debugger("panic")
> > Stopped at  Debugger+0x54:  xchgl   %ebx,in_Debugger.0
> > db>
> >
> > What could possibly cause this?
> > It seem to say that I'm locking an already locked variable.
>
> Hello Jerry,
>
> According to kern/kern_lock.c:lockmgr() your proc/thread already
> has an exclusive lock and it tries to obtain a recursive exclusive
> lock and lock doesn't have LK_CANRECURSE flag (also you allow to
> sleep in lockmng()).
>
> This is documented in lock(9) manual page:
>
>LK_EXCLUSIVEAcquire an exclusive lock.  If an exclusive
>  lock is already held, and LK_CANRECURSE is not
>  set, the system will panic(9).

-- 


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


use after free bugs

2004-02-19 Thread Ted Unangst
Hi.  These are some bugs found by Coverity in a static analysis run on the
FreeBSD kernel.  All these are use after free bugs.




# New errors.
#
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/my/if_my.c|1102|my_detach|ERROR|FREE|1101|1102| Using freed "sc", 
deallocated by call to "free". 
[START_RELAX0=filename|none,fn|free,line1|none,line2|-1,argno|0]

bus_release_resource(dev, SYS_RES_IRQ, 0, sc->my_irq);
bus_release_resource(dev, MY_RES, MY_RID, sc->my_res);
#if 0
contigfree(sc->my_cdata.my_rx_buf, MY_RXBUFLEN + 32, M_DEVBUF);
#endif
Start --->
free(sc, M_DEVBUF);
Error --->
MY_UNLOCK(sc);
splx(s);
mtx_destroy(&sc->my_mtx);
return (0);
}

-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/raidframe/rf_freebsdkintf.c|517|raidctlioctl|ERROR|FREE|516|517| 
Using freed "k_cfg", deallocated by call to "free". 
[START_RELAX0=filename|none,fn|free,line1|none,line2|-1,argno|0]

}
retcode = copyin(k_cfg->layoutSpecific,
(caddr_t) specific_buf,
k_cfg->layoutSpecificSize);
if (retcode) {
Start --->
RF_Free(k_cfg, sizeof(RF_Config_t));
Error --->
RF_Free(specific_buf, 
k_cfg->layoutSpecificSize);
rf_printf(2, "raidctlioctl: retcode=%d "
"copyin.2\n", retcode);
return (retcode);
}
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netsmb/smb_rq.c|732|smb_t2_request_int|ERROR|FREE|730|732| Using freed 
"rqp", deallocated by call to "smb_rq_done". 
[START_RELAX0=filename|/home/tedu/sys/netsmb/smb_rq.c,fn|smb_rq_done,line1|147,line2|-1,argno|0]

md_initm(mdp, mdp->md_top);
}
bad:
smb_iod_removerq(rqp);
freerq:
Start --->
smb_rq_done(rqp);
if (error) {
Error --->
if (rqp->sr_flags & SMBR_RESTART)
t2p->t2_flags |= SMBT2_RESTART;
md_done(&t2p->t2_rparam);
md_done(&t2p->t2_rdata);
}
return error;
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/ips/ips_commands.c|517|ips_ffdc_reset|ERROR|FREE|514|517| Using 
freed "status", deallocated by call to "free". 
[START_RELAX0=filename|none,fn|free,line1|none,line2|-1,argno|0]

status = malloc(sizeof(ips_cmd_status_t), M_DEVBUF, M_NOWAIT|M_ZERO);
if(!status)
return ENOMEM;
if(ips_get_free_cmd(sc, ips_send_ffdc_reset_cmd, status,
IPS_NOWAIT_FLAG)){
Start --->
free(status, M_DEVBUF);
device_printf(sc->dev, "ERROR: unable to get a command! can't send 
ffdc reset!\n");
}
Error --->
if(COMMAND_ERROR(status)){
device_printf(sc->dev, "ERROR: ffdc reset command failed!\n");
}
free(status, M_DEVBUF);
return 0;
}
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/mlx/mlx.c|440|mlx_attach|ERROR|FREE|437|440| Using freed "meo", 
deallocated by call to "free". 
[START_RELAX0=filename|none,fn|free,line1|none,line2|-1,argno|0]

device_printf(sc->mlx_dev, "ENQUIRY_OLD failed\n");
mlx_free(sc);
return(ENXIO);
}
sc->mlx_enq2->me_firmware_id = ('0' << 24) | (0 << 16) | (meo->me_fwminor << 
8) | meo->me_fwmajor;
Start --->
free(meo, M_DEVBUF);

/* XXX require 2.42 or better (PCI) or 2.14 or better (EISA) */
Error --->
if (meo->me_fwminor < 42) {
device_printf(sc->mlx_dev, " *** WARNING *** This firmware revision is not 
recommended\n");
device_printf(sc->mlx_dev, " *** WARNING *** Use revision 2.42 or 
later\n");
}
break;
case MLX_IFTYPE_3:
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/nfsclient/nfs_vfsops.c|509|nfs_mountdiskless|ERROR|FREE|506|509| Double 
free of "nam", deallocated by call to "mountnfs". 
[START_RELAX0=filename|/home/tedu/sys/nfsclient/nfs_vfsops.c,fn|mountnfs,line1|849,line2|-1,argno|2]
 [END_RELAX0=filename|none,fn|free,line1|none,line2|-1,argno|0]

int error;

mp->mnt_kern_flag = 0;
mp->mnt_flag = mountflag;
nam = dup_sockaddr((str

redundant code bugs

2004-02-19 Thread Ted Unangst
These are some more bugs from Coverity.  Most look like typos.


# New errors.
#
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/cam/cam_xpt.c|1885|xptdevicematch|ERROR|NOP| 1885|1885|Operation 
'patterns == 0 || patterns == 0' is a redundant short-circuit or.


/*
 * If there are no match entries, then this device matches no
 * matter what.
 */

Error --->
if ((patterns == NULL) || (patterns == 0))
return(DM_RET_DESCEND | DM_RET_COPY);

for (i = 0; i < num_patterns; i++) {
struct device_match_pattern *cur_pattern;

-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/asr/asr.c|3428|asr_intr|ERROR|NOP| 3428|3428|Operation 'ReplyOffset 
= (*(*sc).ha_Virt).FromFIFO == -1 && ReplyOffset = (*(*sc).ha_Virt).FromFIFO == -1' is 
a redundant short-circuit and.

  processed = 1) {
union asr_ccb * ccb;
U32 ReplyOffset;
PI2O_SCSI_ERROR_REPLY_MESSAGE_FRAME Reply;


Error --->
if (((ReplyOffset = sc->ha_Virt->FromFIFO) == EMPTY_QUEUE)
 && ((ReplyOffset = sc->ha_Virt->FromFIFO) == EMPTY_QUEUE)) {
break;
}
Reply = (PI2O_SCSI_ERROR_REPLY_MESSAGE_FRAME)(ReplyOffset
  - sc->ha_Msgs_Phys + (char *)(sc->ha_Msgs));
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netsmb/smb_trantcp.c|611|smb_nbst_connect|ERROR|NOP| 611|611|Operation 
'(ts2).tv_sec == 0 && (ts2).tv_sec == 0' is a redundant short-circuit and.

error = nb_connect_in(nbp, &sin, td);
if (error)
return error;
getnanotime(&ts2);
timespecsub(&ts2, &ts1);

Error --->
if (ts2.tv_sec == 0 && ts2.tv_sec == 0)
ts2.tv_sec = 1;
nbp->nbp_timo = ts2;
timespecadd(&nbp->nbp_timo, &ts2);
timespecadd(&nbp->nbp_timo, &ts2);
timespecadd(&nbp->nbp_timo, &ts2);  /*  * 4 */
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/contrib/ngatm/netnatm/sig/sig_call.c|3862|uni_sig_call|ERROR|NOP| 
3862|3862|Operation '(*c).cstate == 11 || (*c).cstate == 11' is a redundant 
short-circuit or.

/* Q.2971:Call-Control-U 13/39 (U6) */
/* Q.2971:Call-Control-U 17/39 (U9) */
unx_alerting_request(c, msg, cookie, CALLST_U7);
break;
}

Error --->
if (c->cstate == CALLST_N1 || c->cstate == CALLST_N1) {
/* Q.2971:Call-Control-N 38/39 (N1) */
/* Q.2971:Call-Control-N 7/39  (N3) */
unx_alerting_request(c, msg, cookie, CALLST_N4);
break;
}___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


NULL pointer dereferences

2004-02-19 Thread Ted Unangst
Some more.  There are bugs where the code checks for NULL after using the
pointer.  Usually means the check should be earlier, or is unnecessary.


# New errors.
#
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/twe/twe_freebsd.c|747|twed_dump|ERROR|REVERSE_NULL| 746|747| 
dereference of twed_sc preceeds check! 

int error;
struct disk *dp;

dp = arg;
twed_sc = (struct twed_softc *)dp->d_drv1;
Start --->
twe_sc  = (struct twe_softc *)twed_sc->twed_controller;
Error --->
if (!twed_sc || !twe_sc)
return(ENXIO);

if (length > 0) {
if ((error = twe_dump_blocks(twe_sc, twed_sc->twed_drive->td_twe_unit, offset 
/ TWE_BLOCK_SIZE, virtual, length / TWE_BLOCK_SIZE)) != 0)
return(error);
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netgraph/atm/uni/ng_uni.c|590|uni_uni_output|ERROR|REVERSE_NULL| 
589|590| dereference of msg preceeds check! 
[START_RELAX0=filename|/home/tedu/sys/netgraph/atm/ngatmbase.c,fn|uni_msg_pack_mbuf,line1|152,line2|-1,argno|0]

return;
}
arg.sig = sig;
arg.cookie = cookie;

Start --->
m = uni_msg_pack_mbuf(msg, &arg, sizeof(arg));
Error --->
if (msg != NULL)
uni_msg_destroy(msg);
if (m == NULL)
return;

NG_SEND_DATA_ONLY(error, priv->upper, m);
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/cam/scsi/scsi_da.c|461|daopen|ERROR|REVERSE_NULL| 460|461| dereference 
of periph preceeds check! 

int error;
int s;

s = splsoftcam();
periph = (struct cam_periph *)dp->d_drv1;
Start --->
unit = periph->unit_number;
Error --->
if (periph == NULL) {
splx(s);
return (ENXIO); 
}

softc = (struct da_softc *)periph->softc;
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/trm/trm.c|1341|trm_Interrupt|ERROR|REVERSE_NULL| 1340|1341| 
dereference of pDCB preceeds check! 

return;
}

if (scsi_intstatus & (INT_BUSSERVICE | INT_CMDDONE)) {
pDCB = pACB->pActiveDCB;
Start --->
pSRB = pDCB->pActiveSRB;
Error --->
if (pDCB) {
if (pDCB->DCBFlag & ABORT_DEV_)
trm_EnableMsgOutAbort1(pACB, pSRB);
}
phase = (u_int16_t) pSRB->ScsiPhase;  /* phase: */
stateV = (void *) trm_SCSI_phase0[phase];
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/amr/amr_disk.c|161|amrd_dump|ERROR|REVERSE_NULL| 160|161| 
dereference of amrd_sc preceeds check! 

int error;
struct disk *dp;

dp = arg;
amrd_sc = (struct amrd_softc *)dp->d_drv1;
Start --->
amr_sc  = (struct amr_softc *)amrd_sc->amrd_controller;
Error --->
if (!amrd_sc || !amr_sc)
return(ENXIO);

if (length > 0) {
int driveno = amrd_sc->amrd_drive - amr_sc->amr_drive;
if ((error = amr_dump_blocks(amr_sc,driveno,offset / AMR_BLKSIZE ,(void 
*)virtual,(int) length / AMR_BLKSIZE  )) != 0)
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/kern/uipc_usrreq.c|1097|unp_init|ERROR|REVERSE_NULL| 1096|1097| 
dereference of unp_zone preceeds check! 
[START_RELAX0=filename|/home/tedu/sys/vm/uma_core.c,fn|uma_zone_set_max,line1|1913,line2|-1,argno|0]

void
unp_init(void)
{
unp_zone = uma_zcreate("unpcb", sizeof(struct unpcb), NULL, NULL,
NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE);
Start --->
uma_zone_set_max(unp_zone, nmbclusters);
Error --->
if (unp_zone == 0)
panic("unp_init");
LIST_INIT(&unp_dhead);
LIST_INIT(&unp_shead);
}

-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netgraph/ng_gif.c|270|ng_gif_detach|ERROR|REVERSE_NULL| 268|270| 
dereference of node preceeds check! 
[START_RELAX0=filename|/home/tedu/sys/i386/compile/GENERIC/modules/home/tedu/sys/modules/netgraph/gif/@/netgraph/netgraph.h,fn|_ng_node_private,line1|472,line2|-1,argno|0]

 */
static void
ng_gif_detach(struct ifnet *ifp)
{
const node_p node = IFP2NG(ifp);
Start --->
const priv_p priv = NG_NODE_PRIVATE(node);

Error --->
if (node == NULL)   /* no node (why not?), ignore */
return;
NG_NODE_REALL

size bugs

2004-02-19 Thread Ted Unangst
A few final bugs from Coverity.  Most of these are off by one, the RF bug
is malloc'ing the wrong type.

Thanks for looking.


# New errors.
#
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/twe/twe.c|279|twe_del_unit|ERROR|SIMPLE_BUFFER| 279|279|Accessing 
buffer "(*sc).twe_drive" of size "16" at position "16" with index variable "unit" from 
line 276 [PATH= "unit > 16" on line 276 is false => "unit < 0" on line 276 is false] 

int error;

if (unit < 0 || unit > TWE_MAX_UNITS)
return (ENXIO);


Error --->
if (sc->twe_drive[unit].td_disk == NULL)
return (ENXIO);

error = twe_detach_drive(sc, unit);
return (error);
}
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netinet6/in6.c|1858|in6_prefixlen2mask|ERROR|SIMPLE_BUFFER| 
1858|1858|Accessing buffer "((*maskp).__u6_addr).__u6_addr8" of size "16" at position 
"16" with index variable "bytelen" from line 1853 [PATH= "bitlen != 0" on line 1857 is 
true => "i < bytelen" on line 1855 is false => "i < bytelen" on line 1855 is true] 

bytelen = len / 8;
bitlen = len % 8;
for (i = 0; i < bytelen; i++)
maskp->s6_addr[i] = 0xff;
if (bitlen)

Error --->
maskp->s6_addr[bytelen] = maskarray[bitlen - 1];
}

/*
 * return the best address out of the same scope. if no address was
 * found, return the first valid address from designated IF.
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/netinet6/in6.c|1830|in6_are_prefix_equal|ERROR|SIMPLE_BUFFER| 
1830|1830|Accessing buffer "((*p1).__u6_addr).__u6_addr8" of size "16" at position 
"16" with index variable "bytelen" from line 1825 [PATH= "bcmp != 0" on line 1828 is 
false] 

bytelen = len / 8;
bitlen = len % 8;

if (bcmp(&p1->s6_addr, &p2->s6_addr, bytelen))
return (0);

Error --->
if (p1->s6_addr[bytelen] >> (8 - bitlen) !=
p2->s6_addr[bytelen] >> (8 - bitlen))
return (0);

return (1);
}
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/ata/atapi-cd.c|1633|acd_report_key|ERROR|SIMPLE_BUFFER| 
1632|1633|Accessing buffer "d" of size "0" at position "0" [PATH=] 

ccb[5] = lba & 0xff;
ccb[8] = (length >> 8) & 0xff;
ccb[9] = length & 0xff;
ccb[10] = (ai->agid << 6) | ai->format;

Start --->
d = malloc(length, M_ACD, M_NOWAIT | M_ZERO);
Error --->
d->length = htons(length - 2);

error = ata_atapicmd(cdp->device, ccb, (caddr_t)d, length,
 ai->format == DVD_INVALIDATE_AGID ? 0 : ATA_R_READ,10);
if (error) {
free(d, M_ACD);



# New errors.
#
-
[UNINSPECTED]
X [BUG]
X [FALSE]
X [UNKNOWN]
X [BROKE]
X [SKIP]
/home/tedu/sys/dev/raidframe/rf_diskqueue.c|160|init_dqd|ERROR|SIZE_CHECK| 160|160| 
(*dqd).bp = "malloc"(4 bytes), need 136

static int 
init_dqd(dqd)
RF_DiskQueueData_t *dqd;
{


Error --->
dqd->bp = (RF_Buf_t) malloc(sizeof(RF_Buf_t), M_RAIDFRAME, M_NOWAIT);
if (dqd->bp == NULL) {
return (ENOMEM);
}
memset(dqd->bp, 0, sizeof(RF_Buf_t));   /* if you don't do it, nobody
 * else will.. */___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"