Re: ip_input - chksum - why is it done so early in ip_input?
On Sat, Jan 17, 2004 at 12:50:04AM +0100, Sten Daniel S?rsdal wrote: > > Apologies for the cross-post, i wasnt sure if this was hackers or net material. > > I've often wondered why ip checksumming is done on every incoming > packet and not only on the packets that need to be delivered locally. > It looks like a very expensive way of doing it, especially on high > PPS. Basically all hosts do checksumming so why not just pass the bad > packet on, making the forward process alot cheaper (cpu wise)? It is done this way because the standards demand that it be done this way. RFC1812 says, 4.2.2.5 Header Checksum: RFC 791 Section 3.1 As stated in Section [5.2.2], a router MUST verify the IP checksum of any packet that is received, and MUST discard messages containing invalid checksums. The router MUST NOT provide a means to disable this checksum verification. Keeping a single host from polluting the whole network, and only its LAN, with bad packets is considered worth the cost of every router doing the check. FWIW, this is one of the few places a standard demands that you not even provide the option to disable a feature. -- Crist J. Clark | [EMAIL PROTECTED] | [EMAIL PROTECTED] http://people.freebsd.org/~cjc/| [EMAIL PROTECTED] ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re3: [CHECKER] bugs in FreeBSD
More research... correct me if I am wrong but it appears that the 5.x kmem_malloc() code may have some issues. If you look down at around line 349 there is a comment: /* * Note: if M_NOWAIT specified alone, allocate from * interrupt-safe queues only (just the free list). If * M_USE_RESERVE is also specified, we can also * allocate from the cache. Neither of the latter two * flags may be specified from an interrupt since interrupts * are not allowed to mess with the cache queue. */ if ((flags & (M_NOWAIT|M_USE_RESERVE)) == M_NOWAIT) pflags = VM_ALLOC_INTERRUPT | VM_ALLOC_WIRED; else pflags = VM_ALLOC_SYSTEM | VM_ALLOC_WIRED; Here's the problem... the problem is that malloc(...M_NOWAIT) is used by interrupts not only to avoid blocking, but also to avoid messing with the VM Page 'cache' queue. But in 5.x it is possible for non-interrupt threads to preempt other non-interrupt threads indirectly (due to an interrupt trying to get a mutex that a non-interrupt thread currently holds). Am I correct? But the non-interrupt thread will almost certainly be making memory allocations with M_WAITOK, which means that a preempting thread *CAN* wind up pulling pages out of the 'cache' queue. Now, my understanding is that 5.x's mutexes around the VM system means that this, in fact, will work just fine. So, that means that the above comment is no longer correct, right? In fact, interrupts *should* be able to allocate pages from the VM page 'cache' queue in 5.x now. This leads to the obvious conclusion that 'critical' code, such as the CAM code, which cannot afford to block but which also does terrible things when an M_NOWAIT allocation fails should be able to use (M_WAITOK|M_USE_RESERVE|M_USE_INTERRUPT_RESERVE) and this would result in far safer operation then the current M_NOWAIT use results in. (M_USE_INTERRUPT_RESERVE would be a new M_* flag that allows the system to exhaust the entire free page reserve if necessary and has the same effect as M_NOWAIT had before, but the combination of flags would now allow interrupt-time allocations to also allocate from the cache queue making it virtually impossible for such allocations to fail and that, combined with M_WAITOK, would allow all NULL checks to be removed. It could actually be considered a critical error for the above flags combination to deadlock. -Matt ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
:M_NOWAIT is being used pretty much as if it were M_WAITOK|M_USE_RESERVE :most of the time, especially considering the side effect situation when :such allocations fail. I don't think M_WAITOK|M_USE_RESERVE would be :any less reliable, actually. It looks like the whole paradigm has :shifted away from the original definition of M_NOWAIT to something that :is more like a cross between M_NOWAIT, M_WAITOK, and M_USE_RESERVE. oops, don't take that literally. M_USE_RESERVE means something else. M_NOWAIT is triggering VM_ALLOC_INTERRUPT which is allowed to dig into the free (vm) page reserve. Another interesting thing I've found, and correct me if I'm wrong, but it looks like when the 5.x slab allocator allocates M_NOWAIT memory that newly allocated zone becomes available for normal M_WAITOK allocations as well. This is something DFly's slab allocator does too (I have a big XXX comment for it), and the 4.x allocator too I think. That could create an exhaustion issue. -Matt Matthew Dillon <[EMAIL PROTECTED]> ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
On Sun, 18 Jan 2004, Ruslan Ermilov wrote: > But we're missing the proper NULL checking, here's the fix: ... I've already dealt with this. -- 10 40 80 C0 00 FF FF FF FF C0 00 00 00 00 10 AA AA 03 00 00 00 08 00 ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
Well, this is fun. There are over 460 files in the 5.x source tree (360 in DFly) that make calls to malloc(... M_NOWAIT), and so far about 80% of the calls that I've reviewed generate inappropriate side effects when/if a failure occurs. CAM is the biggest violator... it even has a few panic() conditionals if a malloc(... M_NOWAIT) fails. Not Fun! The only reason it works at all is because M_NOWAIT actually does appear to allow malloc() to block in a number of situations (such as on VM object and map mutexes), and M_NOWAIT triggers VM_ALLOC_INTERRUPT which allows kmem_malloc() to dig into the free page reserve. So in 5.x M_NOWAIT allocations will actually work most of the time.. well, at least until something exhausts the free page reserve at just the wrong time, which is quite possible to do considering how much code is being allowed to dig into the reserve now. M_NOWAIT is being used pretty much as if it were M_WAITOK|M_USE_RESERVE most of the time, especially considering the side effect situation when such allocations fail. I don't think M_WAITOK|M_USE_RESERVE would be any less reliable, actually. It looks like the whole paradigm has shifted away from the original definition of M_NOWAIT to something that is more like a cross between M_NOWAIT, M_WAITOK, and M_USE_RESERVE. This creates a conundrum for me. In DFly M_NOWAIT really means M_NOWAIT, so I am going to have to do something about all the improper M_NOWAIT use in the source base. I'm amazed we haven't had more crashes but even in DFly M_NOWAIT failures due to, e.g. not being able to get the kernel_map lock non-blocking, do not occur all that often. -Matt ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Status GBDE attach at boot
On 2004.01.18 10:19:31 -0500, Allan Fields wrote: > On Sun, Jan 18, 2004 at 02:43:42PM +0100, Simon L. Nielsen wrote: > > On 2004.01.17 14:53:58 -0500, Allan Fields wrote: > > > Hi, > > > > > > I'm interested to know what may be in the pipeline as far as GBDE > > > boot time attach/automation support. Has anyone committed to > > > implementing these features? (I don't see it anymore (on the 5.3 > > > todo list) in releng pages.) > > > > 5.2 already has support for attaching GBDE volumes at boot by using the > > /etc/rc.d/gbde script. I have been using it for a while, and it works > > OK. > > Ahh.. ok, didn't see the changes yet. That is a straight forward > approach - could there just as easily be a similar facility for other > geoms? That shouldn't be a problem... of course depending on exactly you want to configure it might be more or less simple to do. The dependency tree for the rc system can make the script start when needed in the boot sequence without any hacks. Of course the issue of how to set user configuration still exists (as discussed a few times before on the lists), since rc.conf can fast become very cluttered. > > I sent a patch yesterday to the freebsd-rc mailing list make the gbde > > rc.d script work a bit better (see > > http://groups.yahoo.com/group/FreeBSD-rc/message/659 ). > > > > > As a fstab is concerned with mount hack, this is the right approach > > > > I think it's better to just use a rc.d script to attach gbde volumes > > before the normal filesystem mount, since it seems more "clean". Of > > This is good including specifying lockfile dir, but implies passphrase > entry before continuing on always the console? This is the way it works now, but this could be extended. I'm mainly using gbde to encrypt /home on desktops, so asking the password on the console works fine for me. > Which brings us to passphrase from file/filedesc issue vs. from tty > / on command line. Could password prompts be read from another > terminal or from secure source like key device or remote terminal > while the booting continues in the mean-time? I don't see any reason why not, if the "connection" is secure, but I haven't looked into this (since I haven't had the need to) so I'm not exactly sure what kind of problems there are (both programming and security issues). > > course the rc.d script could be enhanced e.g. to support random keys, > > like your "temp" feature. > > Yup. Idea was raised previously on the lists by lucky and phk. > Seems like a good idea for swap,/tmp setup. I actually have an rc.d script by Geoffrey T. Falk <[EMAIL PROTECTED]>, which was posted to some mailing list a few months ago, for gbde swap with random password, but since it confuses the crashdump system I'm not using it right now. -- Simon L. Nielsen FreeBSD Documentation Team pgp0.pgp Description: PGP signature
Re: [CHECKER] bugs in FreeBSD
Matthew Dillon wrote: :> I know cam uses some helper threads so I am not entirely sure about :> the context the cam_sim_alloc() calls are being made in, but if they :> do not create I/O stalls for already-operational SCSI devices then I :> am inclined (in DFly anyway) to simply make the malloc in :> cam_sim_alloc() M_WAITOK. :> :> -Matt :> Matthew Dillon :> <[EMAIL PROTECTED]> :> : :In the 4.x case, so long as the driver doesn't do an splcam() or somehow :block hardware interrupts before calling cam_sim_alloc() you are :probably fine. For 5.x, you might run into Giant problems. : :Scott Well, I don't see how a spl or Giant could possibly have anything to do with memory deadlocks. Both are dropped when a thread blocks so the worst that happens is that you add some latency. CAM doesn't use a kthread in 4.x. It uses it's own SWI hooks. If you call splcam(), then you will block those from running, and no CAM I/O will complete until you call splx(). That's why I say that it's ok to use M_WAITOK so long as you don't block CAM from running. If you want to add a WAITOK/NOWAIT flag parameter to cam_sim_alloc(), that might be a good solution. Scott ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
:> I know cam uses some helper threads so I am not entirely sure about :> the context the cam_sim_alloc() calls are being made in, but if they :> do not create I/O stalls for already-operational SCSI devices then I :> am inclined (in DFly anyway) to simply make the malloc in :> cam_sim_alloc() M_WAITOK. :> :> -Matt :> Matthew Dillon :> <[EMAIL PROTECTED]> :> : :In the 4.x case, so long as the driver doesn't do an splcam() or somehow :block hardware interrupts before calling cam_sim_alloc() you are :probably fine. For 5.x, you might run into Giant problems. : :Scott Well, I don't see how a spl or Giant could possibly have anything to do with memory deadlocks. Both are dropped when a thread blocks so the worst that happens is that you add some latency. The culprit is almost guarenteed to be blocking in the interrupt threads themselves or blocking in serialized multi-device-handling threads such as some of CAM's helper threads. Blocking in either could deadlock the system in a low memory situation. But what people seem to have done... using M_NOWAIT with very little regard for the side effects that occur when malloc() might then fail, is not the right solution. If the CAM code cannot use a blocking malloc for a critical structure allocation then it certainly can't use a non-blocking malloc that might then fail as a workaround! Some other solution is needed for those situations (something like the MPIPE solution I came up with to guarentee the availability of I/O request structures in interrupt service routines). What it comes down to for cam_sim_alloc() is, again, the context in which it is called. Can it be called from a serialized cam thread or an interrupt thread in a way that could potential block I/O operations for devices other then the one trying to attach? If so then there's a real problem that needs to be solved. If not then M_WAITOK can be safely used in this particular situation and the NULL case no longer needs to be worried about. -Matt Matthew Dillon <[EMAIL PROTECTED]> ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
Matthew Dillon wrote: : :Matthew Dillon wrote: :> These cam_sim_alloc() calls seem to be fairly critical to the operation :> of DPT and friends, why is it even possible for them to return NULL :> in the first place and what would be the effect of a (properly handled) :> NULL return if it did occur at this point? :> :> -Matt :> Matthew Dillon :> <[EMAIL PROTECTED]> : : :cam_sim_alloc() is vital to the operation of any CAM driver. However, :cam_sim_alloc() mallocs it's data structures with the M_NOWAIT flag, so :it is possible for it to fail and have to return NULL. The reason it :uses the M_NOWAIT flag is because there is no restrictions on when :driver attach events happen, though they all happen in normal process :or kthread context these days (except at boot, but if you have to sleep :for memory during boot, you have a lot of other problems). : :Scott So, the question becomes: If one were to use M_WAITOK is it possible for a cam_sim_alloc() call for driver A to stall an I/O operation occuring on driver B ? It's the I/O stalls that cause memory deadlocks. Allocations that do not cause I/O stalls on unrelated devices (e.g. your swap) will not cause memory allocation deadlocks. I know cam uses some helper threads so I am not entirely sure about the context the cam_sim_alloc() calls are being made in, but if they do not create I/O stalls for already-operational SCSI devices then I am inclined (in DFly anyway) to simply make the malloc in cam_sim_alloc() M_WAITOK. -Matt Matthew Dillon <[EMAIL PROTECTED]> In the 4.x case, so long as the driver doesn't do an splcam() or somehow block hardware interrupts before calling cam_sim_alloc() you are probably fine. For 5.x, you might run into Giant problems. Scott ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
: :Matthew Dillon wrote: :> These cam_sim_alloc() calls seem to be fairly critical to the operation :> of DPT and friends, why is it even possible for them to return NULL :> in the first place and what would be the effect of a (properly handled) :> NULL return if it did occur at this point? :> :> -Matt :> Matthew Dillon :> <[EMAIL PROTECTED]> : : :cam_sim_alloc() is vital to the operation of any CAM driver. However, :cam_sim_alloc() mallocs it's data structures with the M_NOWAIT flag, so :it is possible for it to fail and have to return NULL. The reason it :uses the M_NOWAIT flag is because there is no restrictions on when :driver attach events happen, though they all happen in normal process :or kthread context these days (except at boot, but if you have to sleep :for memory during boot, you have a lot of other problems). : :Scott So, the question becomes: If one were to use M_WAITOK is it possible for a cam_sim_alloc() call for driver A to stall an I/O operation occuring on driver B ? It's the I/O stalls that cause memory deadlocks. Allocations that do not cause I/O stalls on unrelated devices (e.g. your swap) will not cause memory allocation deadlocks. I know cam uses some helper threads so I am not entirely sure about the context the cam_sim_alloc() calls are being made in, but if they do not create I/O stalls for already-operational SCSI devices then I am inclined (in DFly anyway) to simply make the malloc in cam_sim_alloc() M_WAITOK. -Matt Matthew Dillon <[EMAIL PROTECTED]> ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
Matthew Dillon wrote: These cam_sim_alloc() calls seem to be fairly critical to the operation of DPT and friends, why is it even possible for them to return NULL in the first place and what would be the effect of a (properly handled) NULL return if it did occur at this point? -Matt Matthew Dillon <[EMAIL PROTECTED]> cam_sim_alloc() is vital to the operation of any CAM driver. However, cam_sim_alloc() mallocs it's data structures with the M_NOWAIT flag, so it is possible for it to fail and have to return NULL. The reason it uses the M_NOWAIT flag is because there is no restrictions on when driver attach events happen, though they all happen in normal process or kthread context these days (except at boot, but if you have to sleep for memory during boot, you have a lot of other problems). Scott ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
These cam_sim_alloc() calls seem to be fairly critical to the operation of DPT and friends, why is it even possible for them to return NULL in the first place and what would be the effect of a (properly handled) NULL return if it did occur at this point? -Matt Matthew Dillon <[EMAIL PROTECTED]> :> > * Create the device queue for our SIM. :> > */ :> > Start ---> :> >devq =3D cam_simq_alloc(dpt->max_dccbs); :> >=20 :... :> Index: dpt_scsi.c :> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= :=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= :=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D :> RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v :> retrieving revision 1.45 :> diff -u -p -r1.45 dpt_scsi.c :> --- dpt_scsi.c 24 Aug 2003 17:46:04 - 1.45 :> +++ dpt_scsi.c 18 Jan 2004 15:39:13 - :> @@ -1553,6 +1553,8 @@ dpt_attach(dpt_softc_t *dpt) :> dpt->sims[i] =3D cam_sim_alloc(dpt_action, dpt_poll, "dpt", :> dpt, dpt->unit, /*untagged*/2, :> /*tagged*/dpt->max_dccbs, devq); :> +if (dpt->sims[i] =3D=3D NULL) :> +break; :> if (xpt_bus_register(dpt->sims[i], i) !=3D CAM_SUCCESS) { :> cam_sim_free(dpt->sims[i], /*free_devq*/i =3D=3D 0); :> break; :> %%% :>=20 :Bah, but with this patch that avoids the NULL pointer dereference :we start leaking devq. Attached is a more complete patch, and for :dev/irr/irr.c too. : : :Cheers, :--=20 :Ruslan Ermilov :FreeBSD committer :[EMAIL PROTECTED] : :--yLVHuoLXiP9kZBkt :Content-Type: text/plain; charset=us-ascii :Content-Disposition: attachment; filename=p : :Index: dpt/dpt_scsi.c :=== :RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v :retrieving revision 1.45 :diff -u -p -r1.45 dpt_scsi.c :--- dpt/dpt_scsi.c 24 Aug 2003 17:46:04 - 1.45 :+++ dpt/dpt_scsi.c 18 Jan 2004 15:51:44 - :@@ -1553,6 +1553,11 @@ dpt_attach(dpt_softc_t *dpt) : dpt->sims[i] = cam_sim_alloc(dpt_action, dpt_poll, "dpt", :dpt, dpt->unit, /*untagged*/2, :/*tagged*/dpt->max_dccbs, devq); :+ if (dpt->sims[i] == NULL) { :+ if (i == 0) :+ cam_simq_free(devq); :+ break; :+ } : if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { : cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); : break; :Index: iir/iir.c :=== :RCS file: /home/ncvs/src/sys/dev/iir/iir.c,v :retrieving revision 1.9 :diff -u -p -r1.9 iir.c :--- iir/iir.c 26 Sep 2003 15:36:47 - 1.9 :+++ iir/iir.c 18 Jan 2004 15:52:04 - :@@ -510,6 +510,11 @@ iir_attach(struct gdt_softc *gdt) : gdt->sims[i] = cam_sim_alloc(iir_action, iir_poll, "iir", : gdt, gdt->sc_hanum, /*untagged*/2, : /*tagged*/GDT_MAXCMDS, devq); :+if (gdt->sims[i] == NULL) { :+if (i == 0) :+cam_simq_free(devq); :+break; :+} : if (xpt_bus_register(gdt->sims[i], i) != CAM_SUCCESS) { : cam_sim_free(gdt->sims[i], /*free_devq*/i == 0); : break; : :--yLVHuoLXiP9kZBkt-- : :--SO98HVl1bnMOfKZd :Content-Type: application/pgp-signature :Content-Disposition: inline : :-BEGIN PGP SIGNATURE- :Version: GnuPG v1.2.4 (FreeBSD) : :iD8DBQFACq9iUkv4P6juNwoRAghWAKCBpqGJmtW1g7vOJS15YgKfg/782QCeImr/ :aZ5eUYh2kvOaSBl5zcFd4mE= :=j+I+ :-END PGP SIGNATURE- ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
Looks good to me if you want to commit it, thanks! Scott Ruslan Ermilov wrote: On Sun, Jan 18, 2004 at 02:45:02PM +0200, Ruslan Ermilov wrote: Scott, Attached is the patch that fixes memory leak according to the below report. Attached is the corrected patch that doesn't do a waiting malloc() while interrupts are blocked. Yes I know that splbio() is a no-op these days. ;) On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: - [BUG] though we should demote things that are not locals. /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/ips/ips.c:148:ips_add_waiting_command:ERROR:LEAK:148:154: pointer=waiter from RO=malloc(-1) [s=550,pop=551,pr=1.00] [rank=med] leaked! [z=1.0] [success=5] ips_command_t *command; ips_wait_list_t *waiter; unsigned long memflags = 0; if(IPS_NOWAIT_FLAG & flags) memflags = M_NOWAIT; Start ---> waiter = malloc(sizeof(ips_wait_list_t), M_DEVBUF, memflags); if(!waiter) return ENOMEM; mask = splbio(); if(sc->state & IPS_OFFLINE){ splx(mask); Error ---> return EIO; } command = SLIST_FIRST(&sc->free_cmd_list); if(command && !(sc->state & IPS_TIMEOUT)){ - Cheers, Index: ips.c === RCS file: /home/ncvs/src/sys/dev/ips/ips.c,v retrieving revision 1.6 diff -u -p -r1.6 ips.c --- ips.c 27 Nov 2003 08:37:36 - 1.6 +++ ips.c 18 Jan 2004 13:16:00 - @@ -169,6 +169,7 @@ static int ips_add_waiting_command(ips_s mask = splbio(); if(sc->state & IPS_OFFLINE){ splx(mask); + free(waiter, M_DEVBUF); return EIO; } command = SLIST_FIRST(&sc->free_cmd_list); ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
On Sun, Jan 18, 2004 at 05:44:48PM +0200, Ruslan Ermilov wrote: > On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: > [...] > > - > > [BUG] > > /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/dpt/dpt_scsi.c:1542:dpt_attach:ERROR:LEAK:1542:1571: > > pointer=devq from RO=cam_simq_alloc(-1) [s=21,pop=21,pr=0.99] [rank=med] leaked! > > [z=1.0] [success=3] > > > > int i; > > > > /* > > * Create the device queue for our SIM. > > */ > > Start ---> > > devq = cam_simq_alloc(dpt->max_dccbs); > > > > ... DELETED 23 lines ... > > > > > > } > > if (i > 0) > > EVENTHANDLER_REGISTER(shutdown_final, dptshutdown, > > dpt, SHUTDOWN_PRI_DEFAULT); > > Error ---> > > return (i); > > } > > > > int > > - > > We aren't leaking "devq" here, it's freed (if necessary) by setting > the second cam_sim_free() argument to true: > > if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { > cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); > break; > } > > But we're missing the proper NULL checking, here's the fix: > > %%% > Index: dpt_scsi.c > === > RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v > retrieving revision 1.45 > diff -u -p -r1.45 dpt_scsi.c > --- dpt_scsi.c24 Aug 2003 17:46:04 - 1.45 > +++ dpt_scsi.c18 Jan 2004 15:39:13 - > @@ -1553,6 +1553,8 @@ dpt_attach(dpt_softc_t *dpt) > dpt->sims[i] = cam_sim_alloc(dpt_action, dpt_poll, "dpt", >dpt, dpt->unit, /*untagged*/2, >/*tagged*/dpt->max_dccbs, devq); > + if (dpt->sims[i] == NULL) > + break; > if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { > cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); > break; > %%% > Bah, but with this patch that avoids the NULL pointer dereference we start leaking devq. Attached is a more complete patch, and for dev/irr/irr.c too. Cheers, -- Ruslan Ermilov FreeBSD committer [EMAIL PROTECTED] Index: dpt/dpt_scsi.c === RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v retrieving revision 1.45 diff -u -p -r1.45 dpt_scsi.c --- dpt/dpt_scsi.c 24 Aug 2003 17:46:04 - 1.45 +++ dpt/dpt_scsi.c 18 Jan 2004 15:51:44 - @@ -1553,6 +1553,11 @@ dpt_attach(dpt_softc_t *dpt) dpt->sims[i] = cam_sim_alloc(dpt_action, dpt_poll, "dpt", dpt, dpt->unit, /*untagged*/2, /*tagged*/dpt->max_dccbs, devq); + if (dpt->sims[i] == NULL) { + if (i == 0) + cam_simq_free(devq); + break; + } if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); break; Index: iir/iir.c === RCS file: /home/ncvs/src/sys/dev/iir/iir.c,v retrieving revision 1.9 diff -u -p -r1.9 iir.c --- iir/iir.c 26 Sep 2003 15:36:47 - 1.9 +++ iir/iir.c 18 Jan 2004 15:52:04 - @@ -510,6 +510,11 @@ iir_attach(struct gdt_softc *gdt) gdt->sims[i] = cam_sim_alloc(iir_action, iir_poll, "iir", gdt, gdt->sc_hanum, /*untagged*/2, /*tagged*/GDT_MAXCMDS, devq); +if (gdt->sims[i] == NULL) { +if (i == 0) +cam_simq_free(devq); +break; +} if (xpt_bus_register(gdt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(gdt->sims[i], /*free_devq*/i == 0); break; pgp0.pgp Description: PGP signature
Re: [CHECKER] bugs in FreeBSD
On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: [...] > - > [BUG] > /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/dpt/dpt_scsi.c:1542:dpt_attach:ERROR:LEAK:1542:1571: > pointer=devq from RO=cam_simq_alloc(-1) [s=21,pop=21,pr=0.99] [rank=med] leaked! > [z=1.0] [success=3] > > int i; > > /* >* Create the device queue for our SIM. >*/ > Start ---> > devq = cam_simq_alloc(dpt->max_dccbs); > > ... DELETED 23 lines ... > > > } > if (i > 0) > EVENTHANDLER_REGISTER(shutdown_final, dptshutdown, > dpt, SHUTDOWN_PRI_DEFAULT); > Error ---> > return (i); > } > > int > - We aren't leaking "devq" here, it's freed (if necessary) by setting the second cam_sim_free() argument to true: if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); break; } But we're missing the proper NULL checking, here's the fix: %%% Index: dpt_scsi.c === RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v retrieving revision 1.45 diff -u -p -r1.45 dpt_scsi.c --- dpt_scsi.c 24 Aug 2003 17:46:04 - 1.45 +++ dpt_scsi.c 18 Jan 2004 15:39:13 - @@ -1553,6 +1553,8 @@ dpt_attach(dpt_softc_t *dpt) dpt->sims[i] = cam_sim_alloc(dpt_action, dpt_poll, "dpt", dpt, dpt->unit, /*untagged*/2, /*tagged*/dpt->max_dccbs, devq); + if (dpt->sims[i] == NULL) + break; if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); break; %%% -- Ruslan Ermilov FreeBSD committer [EMAIL PROTECTED] pgp0.pgp Description: PGP signature
Re: Status GBDE attach at boot
On 2004.01.17 14:53:58 -0500, Allan Fields wrote: > Hi, > > I'm interested to know what may be in the pipeline as far as GBDE > boot time attach/automation support. Has anyone committed to > implementing these features? (I don't see it anymore (on the 5.3 > todo list) in releng pages.) 5.2 already has support for attaching GBDE volumes at boot by using the /etc/rc.d/gbde script. I have been using it for a while, and it works OK. I sent a patch yesterday to the freebsd-rc mailing list make the gbde rc.d script work a bit better (see http://groups.yahoo.com/group/FreeBSD-rc/message/659 ). > As a fstab is concerned with mount hack, this is the right approach I think it's better to just use a rc.d script to attach gbde volumes before the normal filesystem mount, since it seems more "clean". Of course the rc.d script could be enhanced e.g. to support random keys, like your "temp" feature. -- Simon L. Nielsen FreeBSD Documentation Team pgp0.pgp Description: PGP signature
Re: network constipation?
On Sun, Jan 18, 2004 at 09:46:33AM +0200, Danny Braniss wrote: > hi, > i have 2 amd64s (one dual opteron, one athlon64), which behave > identicaly. Im trying to compile /usr/ports/x11-fonts, ports & obj are > nfs mounted (on the the same server). > so things go nicely, but after a while gzip hangs, and later > i get 'nfs server dev:/r+d: not responding', this happens on both > hosts, (i just tried it on both to see if the error was in the driver, > since they have different cards). > it's almost obvious that i've hit on a deadlock, where can i look > for some hint? First step when you encounter strange kernel problems is to turn on the debugging options: INVARIANTS, INVARIANTS_SUPPORT, WITNESS etc. Kris pgp0.pgp Description: PGP signature
Status GBDE attach at boot
Hi, I'm interested to know what may be in the pipeline as far as GBDE boot time attach/automation support. Has anyone committed to implementing these features? (I don't see it anymore (on the 5.3 todo list) in releng pages.) As a fstab is concerned with mount hack, this is the right approach I feel. Snapshots use a mount hack too. To do this a small hack to mount(8) with stub for fstype of geom which calls specific command gbde(8) directly or a mount_geom(8) with similar operations to gbde, plus also full geom classes support. # XXX Example fstab of geom entries: # # GBDE swap #/dev/ad0s1bnoneswapsw 0 0 /dev/ad0s1b bde geomrw,attach,temp 0 0 /dev/ad0s1b.bde noneswapsw 0 0 # Normal filesystem /dev/ad0s1a / ufs rw 1 1 /dev/ad0s1f /varufs rw 2 2 /dev/ad0s1g /usrufs rw 2 2 # GBDE tmp dir /dev/ad0s1e bde geomrw,temp 0 0 /dev/ad0s1e.bde /tmpufs rw 0 1 # GBDE home dir (prompt on console; block on ttyin, before getty spawned) #/dev/ad0s1h/dev/ad0s1h.bde geomrw,attach 0 0 #<-long form /dev/ad0s1h bde geomrw,attach 0 0 #<-shorter /dev/ad0s1h.bde /home ufs rw 2 2 # # fs_spec is device # fs_file is GEOM class to instantiate with fs_spec as provider #if using long form split on dot to determine class or specify #class in mntopts (class= or ) # fs_vfstype has new type: ``geom'' # fs_mntops has standard form, plus: # attach: default action for class bde: to attach, so can be omitted # temp: mntopt says init as temporary gbde # init: initialize only # noauto: don't automatically instantiate / attach # level=: level at which to attempt to instantiate geom (def: 1) # 0: in single-user/after root-1: same as noauto # 1: before going multi-user (before getty runs on tty) # 2: after going multi-user (bde needs own tty?) # prompt=: prompt for pass phrases/user input on tty # # insert: insert geom instance (default) #XX remove: remove geom instance - use umount(8) # ...: other class options here # Example prompts on console: # (user can ^C here to skip attaching it) !! GEOM/gbde: Passphrase required for attach of /dev/adNsM.. Enter passphrase: -- GEOM/gbde: Attach sucessful. !! GEOM/gbde: Passphrase required for attach of /dev/da0s1a.. Enter passphrase: gbde: Attach to da0s1a failed: Invalid argument -- GEOM/gbde: Attach failed. -- GEOM/gbde: Done. Jan 17 14:22:03 testhost mount[178]: GEOM/gbde Attach to da0s1a failed: Invalid argument FreeBSD/i386 (testhost) (ttyv0) login: _ Another question, about key entry: should there be an option to allow keys to be read directly from a file/file descriptor instead of on the command line. In this way keys could be piped into the gbde command for attach, etc. from a secure source. Would this prove a significant vulnerability compared to tty input? This might be used in conjunction with other authentication mechanisms and if it proves more secure than -p, could be something to look at. Currently: gbde in readpassphrase(3) prevents reading passphrases on stdin by setting RPP_REQUIRE_TTY and also readpassphrase() isn't designed to accommodate key entry from fd if associated tty. So this would need a command line option to run as a pipeline from an interactive shell. Thanks.. -- Allan Fields BSDCan 2004: May 2004, Ottawa See http://www.bsdcan.org for details. ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CHECKER] bugs in FreeBSD
On Sun, Jan 18, 2004 at 02:45:02PM +0200, Ruslan Ermilov wrote: > Scott, > > Attached is the patch that fixes memory leak according to the below report. > Attached is the corrected patch that doesn't do a waiting malloc() while interrupts are blocked. Yes I know that splbio() is a no-op these days. ;) > On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: > > - > > [BUG] though we should demote things that are not locals. > > /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/ips/ips.c:148:ips_add_waiting_command:ERROR:LEAK:148:154: > > pointer=waiter from RO=malloc(-1) [s=550,pop=551,pr=1.00] [rank=med] leaked! > > [z=1.0] [success=5] > > > > ips_command_t *command; > > ips_wait_list_t *waiter; > > unsigned long memflags = 0; > > if(IPS_NOWAIT_FLAG & flags) > > memflags = M_NOWAIT; > > Start ---> > > waiter = malloc(sizeof(ips_wait_list_t), M_DEVBUF, memflags); > > if(!waiter) > > return ENOMEM; > > mask = splbio(); > > if(sc->state & IPS_OFFLINE){ > > splx(mask); > > Error ---> > > return EIO; > > } > > command = SLIST_FIRST(&sc->free_cmd_list); > > if(command && !(sc->state & IPS_TIMEOUT)){ > > - Cheers, -- Ruslan Ermilov FreeBSD committer [EMAIL PROTECTED] Index: ips.c === RCS file: /home/ncvs/src/sys/dev/ips/ips.c,v retrieving revision 1.6 diff -u -p -r1.6 ips.c --- ips.c 27 Nov 2003 08:37:36 - 1.6 +++ ips.c 18 Jan 2004 13:16:00 - @@ -169,6 +169,7 @@ static int ips_add_waiting_command(ips_s mask = splbio(); if(sc->state & IPS_OFFLINE){ splx(mask); + free(waiter, M_DEVBUF); return EIO; } command = SLIST_FIRST(&sc->free_cmd_list); pgp0.pgp Description: PGP signature
Re: [CHECKER] bugs in FreeBSD
Scott, Attached is the patch that fixes memory leak according to the below report. On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: > - > [BUG] though we should demote things that are not locals. > /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/ips/ips.c:148:ips_add_waiting_command:ERROR:LEAK:148:154: > pointer=waiter from RO=malloc(-1) [s=550,pop=551,pr=1.00] [rank=med] leaked! > [z=1.0] [success=5] > > ips_command_t *command; > ips_wait_list_t *waiter; > unsigned long memflags = 0; > if(IPS_NOWAIT_FLAG & flags) > memflags = M_NOWAIT; > Start ---> > waiter = malloc(sizeof(ips_wait_list_t), M_DEVBUF, memflags); > if(!waiter) > return ENOMEM; > mask = splbio(); > if(sc->state & IPS_OFFLINE){ > splx(mask); > Error ---> > return EIO; > } > command = SLIST_FIRST(&sc->free_cmd_list); > if(command && !(sc->state & IPS_TIMEOUT)){ > - Cheers, -- Ruslan Ermilov FreeBSD committer [EMAIL PROTECTED] Index: ips.c === RCS file: /home/ncvs/src/sys/dev/ips/ips.c,v retrieving revision 1.6 diff -u -p -r1.6 ips.c --- ips.c 27 Nov 2003 08:37:36 - 1.6 +++ ips.c 18 Jan 2004 12:41:22 - @@ -163,14 +163,14 @@ static int ips_add_waiting_command(ips_s unsigned long memflags = 0; if(IPS_NOWAIT_FLAG & flags) memflags = M_NOWAIT; - waiter = malloc(sizeof(ips_wait_list_t), M_DEVBUF, memflags); - if(!waiter) - return ENOMEM; mask = splbio(); if(sc->state & IPS_OFFLINE){ splx(mask); return EIO; } + waiter = malloc(sizeof(ips_wait_list_t), M_DEVBUF, memflags); + if(!waiter) + return ENOMEM; command = SLIST_FIRST(&sc->free_cmd_list); if(command && !(sc->state & IPS_TIMEOUT)){ SLIST_REMOVE_HEAD(&sc->free_cmd_list, next); pgp0.pgp Description: PGP signature
network constipation?
hi, i have 2 amd64s (one dual opteron, one athlon64), which behave identicaly. Im trying to compile /usr/ports/x11-fonts, ports & obj are nfs mounted (on the the same server). so things go nicely, but after a while gzip hangs, and later i get 'nfs server dev:/r+d: not responding', this happens on both hosts, (i just tried it on both to see if the error was in the driver, since they have different cards). it's almost obvious that i've hit on a deadlock, where can i look for some hint? thanks, danny ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"