Jim Meyering wrote: > Paul Eggert wrote: >> Could you please try this little patch? It should fix your >> problem. I came up with this fix in my sleep (literally! >> I woke up this morning and the patch was in my head), but >> haven't had time to look at the code in this area to see >> if it's the best fix. >> >> Clearly there's at least one more bug as noted in my previous email >> <http://lists.gnu.org/archive/html/bug-coreutils/2010-11/msg00216.html> >> but I expect it's less likely to fire. >> >> diff --git a/src/sort.c b/src/sort.c >> index 7e25f6a..1aa1eb4 100644 >> --- a/src/sort.c >> +++ b/src/sort.c >> @@ -3226,13 +3226,13 @@ queue_pop (struct merge_node_queue *queue) >> static void >> write_unique (struct line const *line, FILE *tfp, char const *temp_output) >> { >> - static struct line const *saved = NULL; >> + static struct line saved; >> >> if (!unique) >> write_line (line, tfp, temp_output); >> - else if (!saved || compare (line, saved)) >> + else if (!saved.text || compare (line, &saved)) >> { >> - saved = line; >> + saved = *line; >> write_line (line, tfp, temp_output); >> } >> } > > Nice. > That looks right to me. > > FYI, what happens is the first fillbuf call gets a block > of lines, and "saved" ends up pointing at one of them. > The second fillbuf's fread races to overwrite a byte or two of the > saved->text pointer that is being dereferenced by the other thread. ...
Hi Paul, I'm getting ready to push something like the following (I'll add a NEWS entry first, though) Is there anything you'd like to add? >From 46589f6be5ce6cd7ff474059a33f47b57094ff21 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Tue, 30 Nov 2010 22:30:12 +0100 Subject: [PATCH] sort -u: fix a thread-race pointer corruption bug * src/sort.c (write_unique): Save the entire "struct line", not just a pointer to one. Otherwise, with a multi-thread run, sometimes, with some inputs, fillbuf would would win a race and clobber a "saved->text" pointer in one thread just before it was dereferenced in a comparison in another thread. --- src/sort.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sort.c b/src/sort.c index 7e25f6a..1aa1eb4 100644 --- a/src/sort.c +++ b/src/sort.c @@ -3226,13 +3226,13 @@ queue_pop (struct merge_node_queue *queue) static void write_unique (struct line const *line, FILE *tfp, char const *temp_output) { - static struct line const *saved = NULL; + static struct line saved; if (!unique) write_line (line, tfp, temp_output); - else if (!saved || compare (line, saved)) + else if (!saved.text || compare (line, &saved)) { - saved = line; + saved = *line; write_line (line, tfp, temp_output); } } -- 1.7.3.2.846.gf4b062
