You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/10/31 16:02:20 UTC
Re: svn commit: r1404159 - /subversion/trunk/subversion/libsvn_subr/named_atomic.c
stefan2@apache.org writes:
> Author: stefan2
> Date: Wed Oct 31 14:19:59 2012
> New Revision: 1404159
>
> URL: http://svn.apache.org/viewvc?rev=1404159&view=rev
> Log:
> "Harden" our named atomics against data file corruption. Even if
> the memory block contains completely random data, we shall never
> see an access outside that buffer.
> + /* Sanitize (in case of data corruption)
> + */
> + if (new_ns->data->count > MAX_ATOMIC_COUNT)
> + new_ns->data->count = MAX_ATOMIC_COUNT;
I'm still seeing a crash:
467 if (new_ns->data->count > MAX_ATOMIC_COUNT)
(gdb) p new_ns->data->count
$1 = -1382404098
I suppose we could either test "count < 0" or make count unsigned?
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1404159 - /subversion/trunk/subversion/libsvn_subr/named_atomic.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Oct 31, 2012 at 5:38 PM, Julian Foad <ju...@btopenworld.com>wrote:
> Philip Martin:
>
> > stefan2@apache.org writes:
> >> + /* Sanitize (in case of data corruption)
> >> + */
> >> + if (new_ns->data->count > MAX_ATOMIC_COUNT)
> >> + new_ns->data->count = MAX_ATOMIC_COUNT;
> >
> > I'm still seeing a crash:
> >
> > 467 if (new_ns->data->count > MAX_ATOMIC_COUNT)
> > (gdb) p new_ns->data->count
> > $1 = -1382404098
>
> Also, if the count is "corrupted", I want to ask: are we sure it is then
> safe to adjust the count and carry on? (I haven't been paying close
> attention, I'm just asking.)
>
You are right. When we detect that someone messed
with our on-disk data, we should simply bail out instead
of trying to limp on. svnadmin recover is the place to
try to fix things (which we do since r1404163).
Changed in r1405772.
-- Stefan^2.
--
Certified & Supported Apache Subversion Downloads:
*
http://www.wandisco.com/subversion/download
*
Re: svn commit: r1404159 - /subversion/trunk/subversion/libsvn_subr/named_atomic.c
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin:
> stefan2@apache.org writes:
>> + /* Sanitize (in case of data corruption)
>> + */
>> + if (new_ns->data->count > MAX_ATOMIC_COUNT)
>> + new_ns->data->count = MAX_ATOMIC_COUNT;
>
> I'm still seeing a crash:
>
> 467 if (new_ns->data->count > MAX_ATOMIC_COUNT)
> (gdb) p new_ns->data->count
> $1 = -1382404098
Also, if the count is "corrupted", I want to ask: are we sure it is then safe to adjust the count and carry on? (I haven't been paying close attention, I'm just asking.)
- Julian