Bug 27990
| Summary: | ypbind uses memory after freeing it | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | ben <ben> | ||||
| Component: | bin | Assignee: | ben <ben> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 4.3-STABLE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
ben@FreeBSD.org writes: > >Fix: > > This has fixed the problem for me on -stable, not sure if it applies to > current too or not though. If someone can just review it I can commit > it. It applies to -current. I've tried it and it works fine. I also agree with your analysis of the problem. > > --- ypbind.c.orig Sat Jun 9 17:03:47 2001 > +++ ypbind.c Sat Jun 9 17:04:32 2001 > @@ -394,7 +394,7 @@ > int i; > DIR *dird; > struct dirent *dirp; > - struct _dom_binding *ypdb; > + struct _dom_binding *ypdb, *next; > > /* Check that another ypbind isn't already running. */ > if ((yplockfd = (open(YPBINDLOCK, O_RDONLY|O_CREAT, 0444))) == -1) > @@ -493,7 +493,8 @@ > syslog(LOG_WARNING, "select: %m"); > break; > default: > - for(ypdb=ypbindlist; ypdb; ypdb=ypdb->dom_pnext) { > + for(ypdb=ypbindlist; ypdb; ypdb=next) { > + next = ypdb->dom_pnext; > if (READFD > 0 && FD_ISSET(READFD, &fdsr)) { > handle_children(ypdb); > if (children == (MAX_CHILDREN - 1)) > > >Release-Note: > >Audit-Trail: > >Unformatted: > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-bugs" in the body of the message > [ cc'd to phk as my mentor for src commits... ] could someone please review this fairly simple patch for me? If someone could test it on -current too that would be great, I don't have a -current machine at the moment. It works fine on -stable though. It's had one review already but a few more can't hurt. http://www.FreeBSD.org/cgi/query-pr.cgi?pr=27990 -- Ben Smithurst / ben@FreeBSD.org Looks good to my eyes, but I'm not able to test it. Poul-Henning In message <20010615125602.A31582@comp.leeds.ac.uk>, Ben Smithurst writes: >[ cc'd to phk as my mentor for src commits... ] > >could someone please review this fairly simple patch for me? If someone >could test it on -current too that would be great, I don't have a >-current machine at the moment. It works fine on -stable though. It's >had one review already but a few more can't hurt. > >http://www.FreeBSD.org/cgi/query-pr.cgi?pr=27990 > >-- >Ben Smithurst / ben@FreeBSD.org > -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. Responsible Changed From-To: freebsd-bugs->ben Committed to -current, will MFC in 2 weeks if no problems arise. State Changed From-To: open->closed committed in -current and 4-stable. |
Maybe I'm missing something but this code just looks plain Wrong to me: for(ypdb=ypbindlist; ypdb; ypdb=ypdb->dom_pnext) { if (READFD > 0 && FD_ISSET(READFD, &fdsr)) { handle_children(ypdb); if (children == (MAX_CHILDREN - 1)) checkwork(); } } Given this code in the handle_children() function: switch(ypdb->dom_default) { case 0: ... free(ypdb); << HERE domains--; return; After returning the main() function will do 'ypdb=ypdb->dom_pnext' after 'ypdb' has been freed. Fix: This has fixed the problem for me on -stable, not sure if it applies to current too or not though. If someone can just review it I can commit it. How-To-Repeat: Not quite sure. I noticed it while I had one machine on my network using the old NIS domain after I'd changed the rest to a new one, maybe that's related.