Bug #56649 for File-Map: negative offset segv

Thu Apr 15 20:07:39 2010 user42 [...] zip.com.au - Ticket created

Subject:	negative offset segv
Date:	Fri, 16 Apr 2010 10:06:27 +1000
To:	bug-File-Map [...] rt.cpan.org
From:	Kevin Ryde <user42 [...] zip.com.au>

With 0.24 a small negative offset like map_file (my $str, '/etc/motd', '<', -1, 100); print $str,"\n"; => segv succeeds in the map_file and then segfaults printing the string, where I expected it to fail with EINVAL the same as a large negative does, map_file (my $str, '/etc/motd', '<', -9999, 100); => EINVAL I think the page-boundary "correction" value goes negative if the input is negative, which I think rounds the wrong way. Eg. offset==-1 length==100 becomes offset==0 length==99 or something like that, where it probably should be offset==-4096 length==4195 of some such. It's probably both easiest and best to let negatives go through to the system call, so it can give whatever the usual system error is for such nonsense -- but take care not to turn negative into non-negative by the pagesize rounding :-).

Thu Apr 15 20:12:47 2010 LEONT [...] cpan.org - Taken

Thu Apr 15 20:14:14 2010 LEONT [...] cpan.org - Severity Critical added

Thu Apr 15 20:15:31 2010 LEONT [...] cpan.org - Status changed from 'new' to 'open'

Thu Apr 15 21:03:55 2010 LEONT [...] cpan.org - Correspondence added

This is obviously a serious bug. I think the cleanest way to solve it would be to make negative values for offset DWIM: -100 means 100 bytes from the end. That way not only a bug would be squashed, but also a feature added ;-). On Thu Apr 15 20:07:39 2010, user42@zip.com.au wrote: Show quoted text

> With 0.24 a small negative offset like > > map_file (my $str, '/etc/motd', '<', -1, 100); > print $str,"\n"; > => segv > > succeeds in the map_file and then segfaults printing the string, where I > expected it to fail with EINVAL the same as a large negative does, > > map_file (my $str, '/etc/motd', '<', -9999, 100); > => EINVAL > > I think the page-boundary "correction" value goes negative if the input > is negative, which I think rounds the wrong way. Eg. offset==-1 > length==100 becomes offset==0 length==99 or something like that, where > it probably should be offset==-4096 length==4195 of some such. > > It's probably both easiest and best to let negatives go through to the > system call, so it can give whatever the usual system error is for such > nonsense -- but take care not to turn negative into non-negative by the > pagesize rounding :-).

Sat Apr 17 22:01:59 2010 user42 [...] zip.com.au - Correspondence added

Subject:	Re: [rt.cpan.org #56649] negative offset segv
Date:	Sun, 18 Apr 2010 12:01:32 +1000
To:	bug-File-Map [...] rt.cpan.org
From:	Kevin Ryde <user42 [...] zip.com.au>

"Leon Timmermans via RT" <bug-File-Map@rt.cpan.org> writes: Show quoted text

> > make negative values for offset DWIM: -100 means 100 bytes > from the end.

That'd be a possibility. It feels slightly like a departure from the basic mmap(). I suppose it'd also be only the offset as of the current file size, not tracking later increase or decrease. Perhaps that's what you'd expect. Maybe start by throwing EINVAL for all negatives, and ponder the alternatives a bit. (Or if you go UV for the off_t conversion then negatives won't arise anyway in the 64-bit off_t mode :-)

Tue Apr 20 08:31:20 2010 LEONT [...] cpan.org - Correspondence added

On Sat Apr 17 22:01:59 2010, user42@zip.com.au wrote: Show quoted text

> That'd be a possibility. It feels slightly like a departure from the > basic mmap().

Departing from mmap is not a problem. Its interface is rather abysmal. A more perlish, substr-like interface wouldn't be a bad idea. Show quoted text

> I suppose it'd also be only the offset as of the current > file size, not tracking later increase or decrease.

That would be fairly impossible to do. Show quoted text

> Maybe start by throwing EINVAL for all negatives, and

ponder the alternatives a bit. True that. I've just uploaded a new version that throws an error.

Tue Apr 20 08:31:21 2010 LEONT [...] cpan.org - Status changed from 'open' to 'resolved'