Today's patches

[Richard Kettlewell]

Julien ÉLIE <julien at trigofacile.com> writes:
> Hi Richard,
>
>> /* A magic number for the group.index file so that we can later change the
>>    format in a backward-compatible fashion. */
>> #define TDX_MAGIC       (~(0xf1f0f33d))
>> 
>> Why is it defined in the inverse?  i.e. why not the much clearer:
>> 
>> #define TDX_MAGIC 0x0e0f0cc2
>
> I admit I do not know.  Leaving this as-is does no harm, though.

*nod*

>> Subject: [PATCH 2/3] Remove redundant (broken!) code
>> 
>> The check was (i) off by one and (ii) can never happen, given the
>> loop condition.
>
> At the end of the loop, we have:
>
>         parent = &entry->next.recno;
>         current = *parent;

Yes - but that is too late for the call to entry_splice(), which is
where the user-after-munmap (or use-after-free) would occur.  So parent
must be recomputed somehow if a remap occurs.  The conservative way to
do it would be just to start again from the top.

> Can't we imagine that the group index still has not been remaped
> after the creation of a new newsgroup (therefore situated at index->count)?
> Then, index->count is the next.recno and we should check that the same way
> you did for patch #3.
>
> "while (current >= 0)" only and check whether "current >= index->count"?
>
> Wouldn't it make sense to do that?

I don't know if the situation is a realistic one - I've not accumulated
enough whole-system knowledge about INN yet.  If it is then your
approach is surely the right one.

-- 
http://www.greenend.org.uk/rjk/