projects / irc.git / blame
| Commit | Line | Data |
|---|---|---|
| 3bd189cb JR |
1 | <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN"> |
| 2 | <HTML> | |
| 3 | <HEAD> | |
| 4 | <TITLE> [IRCServices] Allocation error: ns unsuspend [retry] | |
| 5 | </TITLE> | |
| 6 | <LINK REL="Index" HREF="index.html" > | |
| 7 | <LINK REL="made" HREF="mailto:ircservices%40ircservices.za.net?Subject=%5BIRCServices%5D%20Allocation%20error%3A%20ns%20unsuspend%20%5Bretry%5D&In-Reply-To=20040726120549.PHYF11275.mta09-svc.ntlworld.com%40excelsior"> | |
| 8 | <META NAME="robots" CONTENT="index,nofollow"> | |
| 9 | <META http-equiv="Content-Type" content="text/html; charset=us-ascii"> | |
| 10 | <LINK REL="Previous" HREF="004539.html"> | |
| 11 | <LINK REL="Next" HREF="004541.html"> | |
| 12 | </HEAD> | |
| 13 | <BODY BGCOLOR="#ffffff"> | |
| 14 | <H1>[IRCServices] Allocation error: ns unsuspend [retry]</H1> | |
| 15 | <B>Andrew Church</B> | |
| 16 | <A HREF="mailto:ircservices%40ircservices.za.net?Subject=%5BIRCServices%5D%20Allocation%20error%3A%20ns%20unsuspend%20%5Bretry%5D&In-Reply-To=20040726120549.PHYF11275.mta09-svc.ntlworld.com%40excelsior" | |
| 17 | TITLE="[IRCServices] Allocation error: ns unsuspend [retry]">achurch at achurch.org | |
| 18 | </A><BR> | |
| 19 | <I>Mon Jul 26 22:29:50 PDT 2004</I> | |
| 20 | <P><UL> | |
| 21 | <LI>Previous message: <A HREF="004539.html">[IRCServices] Allocation error: ns unsuspend [retry] | |
| 22 | </A></li> | |
| 23 | <LI>Next message: <A HREF="004541.html">[IRCServices] Allocation error: ns unsuspend [retry] | |
| 24 | </A></li> | |
| 25 | <LI> <B>Messages sorted by:</B> | |
| 26 | <a href="date.html#4540">[ date ]</a> | |
| 27 | <a href="thread.html#4540">[ thread ]</a> | |
| 28 | <a href="subject.html#4540">[ subject ]</a> | |
| 29 | <a href="author.html#4540">[ author ]</a> | |
| 30 | </LI> | |
| 31 | </UL> | |
| 32 | <HR> | |
| 33 | <!--beginarticle--> | |
| 34 | <PRE> I did see your original message--it's just that I've been going | |
| 35 | through the entire code to find and fix similar bugs as well. I have a | |
| 36 | sneaking suspicion that the expire-on-get design plays nasty games with | |
| 37 | pointers in the user record too, but as it hasn't seemed to cause any | |
| 38 | problems so far, I'm saving that bit of contemplation for 5.1 (which, | |
| 39 | incidentally, will also have similar use-after-free checks in the memory | |
| 40 | checking code). | |
| 41 | ||
| 42 | At any rate, thanks for the report. | |
| 43 | ||
| 44 | --Andrew Church | |
| 45 | <A HREF="http://www.ircservices.za.net/mailman/listinfo/ircservices">achurch at achurch.org</A> | |
| 46 | <A HREF="http://achurch.org/">http://achurch.org/</A> | |
| 47 | ||
| 48 | ><i>Hi again, | |
| 49 | </I>><i> | |
| 50 | </I>><i>[ I sent this yesterday, but it seems it got put under another thread in the | |
| 51 | </I>><i>archive, and didn't actually get sent out to the list, so I'll send it again | |
| 52 | </I>><i>in case it gets missed :) ] | |
| 53 | </I>><i> | |
| 54 | </I>><i>I just noticed and patched another bug, quite similar to the last issue I | |
| 55 | </I>><i>posted about ("Seg Fault/Bus Error on SQUIT": | |
| 56 | </I>><i><A HREF="http://www.ircservices.za.net/pipermail/ircservices/2004/003377.html">http://www.ircservices.za.net/pipermail/ircservices/2004/003377.html</A> ). | |
| 57 | </I>><i> | |
| 58 | </I>><i>Again, I noticed this one because free() is set up to write a pattern over | |
| 59 | </I>><i>memory as it is released on our services host. It wouldn't occur in most | |
| 60 | </I>><i>normal circumstances, but might do unpredictably, depending on a lot of | |
| 61 | </I>><i>factors, so ought to be fixed. | |
| 62 | </I>><i> | |
| 63 | </I>><i>The problem occurs when unsuspending a nickname that is part of a group in | |
| 64 | </I>><i>which no nickname has been used for longer than the NSExpire setting - or | |
| 65 | </I>><i>something along those lines. | |
| 66 | </I>><i> | |
| 67 | </I>><i>In modules/nickserv/util.c, unsuspend_nick() does this: | |
| 68 | </I>><i> | |
| 69 | </I>><i> ARRAY_FOREACH (i, ngi->nicks) { | |
| 70 | </I>><i> NickInfo *ni = get_nickinfo(ngi->nicks[i]); | |
| 71 | </I>><i> ... | |
| 72 | </I>><i> } | |
| 73 | </I>><i> | |
| 74 | </I>><i>get_nickinfo() will free (NickGroupInfo *)ngi under certain conditions | |
| 75 | </I>><i>(roughly as I described above), making the following attempts to dereference | |
| 76 | </I>><i>ngi crash the program: | |
| 77 | </I>><i> | |
| 78 | </I>><i> if (!ni) { | |
| 79 | </I>><i> module_log("unsuspend: unable to retrieve NickInfo | |
| 80 | </I>><i>for %s" | |
| 81 | </I>><i> " (nick group %u)", ngi->nicks[i], ngi->id); | |
| 82 | </I>><i> continue; | |
| 83 | </I>><i> } | |
| 84 | </I>><i> | |
| 85 | </I>><i>ngi would also be used in subsequent loops, so just changing the log message | |
| 86 | </I>><i>wouldn't be a solution. | |
| 87 | </I>><i> | |
| 88 | </I>><i>I didn't have a lot of time to investigate the best way to fix this, but | |
| 89 | </I>><i>here is the patch I came up with. It seems to do the job, but I would be | |
| 90 | </I>><i>grateful if anyone can advise something more suitable. (Lines beginning "+" | |
| 91 | </I>><i>were added; noexpire is just set to 1 before the loop, and restored | |
| 92 | </I>><i>afterwards, which stops the expiry check. Note that the NSSuspendGrace stuff | |
| 93 | </I>><i>only happens a few lines after the call to get_nickinfo(), so for that small | |
| 94 | </I>><i>time, nick groups can disappear.). | |
| 95 | </I>><i> | |
| 96 | </I>><i> void unsuspend_nick(NickGroupInfo *ngi, int set_time) | |
| 97 | </I>><i> { | |
| 98 | </I>><i> time_t now = time(NULL); | |
| 99 | </I>><i>+ int cache_noexpire = 0; | |
| 100 | </I>><i> | |
| 101 | </I>><i> if (!ngi->suspendinfo) { | |
| 102 | </I>><i> module_log("unsuspend: called on non-suspended nick group %u [%s]", | |
| 103 | </I>><i> | |
| 104 | </I>><i>... | |
| 105 | </I>><i> | |
| 106 | </I>><i> " %u) to %ld", ngi->nicks[ngi->mainnick], ngi->id, | |
| 107 | </I>><i> (long)ngi->authset); | |
| 108 | </I>><i> } | |
| 109 | </I>><i>+ cache_noexpire = noexpire; | |
| 110 | </I>><i>+ noexpire = 1; | |
| 111 | </I>><i> ARRAY_FOREACH (i, ngi->nicks) { | |
| 112 | </I>><i> NickInfo *ni = get_nickinfo(ngi->nicks[i]); | |
| 113 | </I>><i> if (!ni) { | |
| 114 | </I>><i> | |
| 115 | </I>><i>... | |
| 116 | </I>><i> | |
| 117 | </I>><i> } | |
| 118 | </I>><i> } | |
| 119 | </I>><i> } | |
| 120 | </I>><i>+ noexpire = cache_noexpire; | |
| 121 | </I>><i> } | |
| 122 | </I>><i> | |
| 123 | </I>><i> | |
| 124 | </I>><i>/*************************************************************************/ | |
| 125 | </I>><i> | |
| 126 | </I>><i>-- | |
| 127 | </I>><i>Tom McIntyre | |
| 128 | </I>><i><A HREF="http://www.ircservices.za.net/mailman/listinfo/ircservices">chawmp at cyberarmy.net</A> | |
| 129 | </I>><i> | |
| 130 | </I>><i> | |
| 131 | </I>><i> | |
| 132 | </I>><i>------------------------------------------------------------------ | |
| 133 | </I>><i>To unsubscribe or change your subscription options, visit: | |
| 134 | </I>><i><A HREF="http://www.ircservices.za.net/mailman/listinfo/ircservices">http://www.ircservices.za.net/mailman/listinfo/ircservices</A> | |
| 135 | </I> | |
| 136 | ||
| 137 | </PRE> | |
| 138 | ||
| 139 | <!--endarticle--> | |
| 140 | <HR> | |
| 141 | <P><UL> | |
| 142 | <!--threads--> | |
| 143 | <LI>Previous message: <A HREF="004539.html">[IRCServices] Allocation error: ns unsuspend [retry] | |
| 144 | </A></li> | |
| 145 | <LI>Next message: <A HREF="004541.html">[IRCServices] Allocation error: ns unsuspend [retry] | |
| 146 | </A></li> | |
| 147 | <LI> <B>Messages sorted by:</B> | |
| 148 | <a href="date.html#4540">[ date ]</a> | |
| 149 | <a href="thread.html#4540">[ thread ]</a> | |
| 150 | <a href="subject.html#4540">[ subject ]</a> | |
| 151 | <a href="author.html#4540">[ author ]</a> | |
| 152 | </LI> | |
| 153 | </UL> | |
| 154 | ||
| 155 | </body></html> |