[Pkg-chromium-maint] Bug#660187: chromium-browser: Entry #3 in data pack points off end of file. Was the file corrupted?

[shawn]

On Wed, 2012-06-13 at 01:09 -0500, Jonathan Nieder wrote: 
> shawn wrote:
> 
> > no it is not certain, sorry for being negative. Patch below for that issue. However
> > I am unable to continue working on this, because I do not know how to run
> > chromium from the build directory directly, without installing (root access).
> > Can you help with that?
> 
> Sure --- just using the chromium binary from the build tree with no
> special preparation works fine for me.  What happens when you try it?
> 
> > This is only run at startup, so there are other unaligned memory access issues.
> > (the logs do spam if you have reporting on)
> 
> Thanks.  A list would be nice, so interested people can try to make it
> shorter.  If it's very big, feel free to send me a private email so I
> can cut it down to one entry per code address.
> 
> > commit e4f2ddb3fc2333bae34fd8bd9552933ab698bd95
> > Author: Shawn Landden <shawnlandden at gmail.com>
> > Date:   Sun Jun 3 23:42:30 2012 -0700
> >
> >     fix unalignment memory access in src/ui/base/resource/data_pack.cc:132
> > 
> > diff --git a/src/ui/base/resource/data_pack.cc
> > b/src/ui/base/resource/data_pack.cc
> > index be44afa..e2b37f1 100644
> > --- a/src/ui/base/resource/data_pack.cc
> > +++ b/src/ui/base/resource/data_pack.cc
> > @@ -129,7 +129,8 @@ bool DataPack::Load(const FilePath& path) {
> >    for (size_t i = 0; i < resource_count_ + 1; ++i) {
> >      const DataPackEntry* entry = reinterpret_cast<const
> > DataPackEntry*>(
> >          mmap_->data() + kHeaderLength + (i * sizeof(DataPackEntry)));
> > -    if (entry->file_offset > mmap_->length()) {
> > +    // file_offset is 4-byte, but only aligned to 2-byte boundry
> > +    if (memcmp(entry->file_offset, mmap_->length(), 32) > 0) {
> 
> I guess I'd suggest
> 
> 	namespace {
> 
> 	uint32 get_unaligned_32(const void *addr)
> 	{
> 		uint32 result;
> 		memcpy(result, addr, sizeof(result);
> 		return result;
> 	}
> 
> 	...
> 	// no need to use pragma pack
> 	struct DataPackEntry {
> 	  uint16 resource_id;
> 	  uint8 raw_file_offset[4];
> 
> 	  uint32 file_offset() {
> 	    return get_unaligned_32(raw_file_offset);
> 	  }
> 	  ...
> 	}
I wrote up a bunch of stuff in response to this. But well, its just a
bikeshed issue to me. This is debug code, that was added to debug
problems loading the file on windows, here that likely does not belong
in the optimized builds in the first place. And why do a copy when you
don't have to? 
> 
> 	}
> 	...
> 
> 	if (entry->file_offset()) > mmap_->length()) {
> 
> 	...
> 	size_t length = next_entry->file_offset() - target->file_offset();
> 
> Many thanks,
> Jonathan


-- 
-Shawn Landden