Re: com php-src: Partial revert of 1a7798c786466977ef8d2bc95b40a435c47c845a that introduced wrong memory access: ext/fileinfo/libmagic/softmagic.c

Hi Anatol, two new fileinfo tests provided by Stas reported valgrind errors. they work fine on 5.5 and 5.6. Thanks. Dmitry. On Tue, Apr 14, 2015 at 5:14 PM, Anatol Belski <anatol@belski.net> wrote: > Hi Dmitry, > > > > Basically the merge was to upgrade to libmagic 5.22, also several other > patches was applied. The particular piece of that patch was introduced in > libmagic 5.22 https://github.com/file/file/commit/defbf885 , so > this is > not a merge mistake. Since then there was one bug in this patched version > which was fixed. Otherwise vargrind was green with all available tests. > > > > So this revert turns the part from libmagic around 5.16 back. Looks a bit > strange as actually that’s not a merge mistake. I guess that some bad thing > could be happening around lines 631-638 if ms->search.s had some bad data > (for whatever reasons). > > > > Maybe you could give me a reproducer so I could play with it a bit more? > Maybe also should bisect to at least to be sure it wasn’t affected by some > other change. If it’ hard to reproduce, we probably can live with this > partial revert. The libmagic in PHP is anyway an odd modified thing J , > just it were easier to keep the diff as small as possible. Even more as we > have to upgrade it, this time it was a jump over about 5 releases (and > probably we should catch the 5.23 if it’s out propitious with PHP7 timing, > as it’ll stay in maybe for the whole first minor). > > > > Regards > > > > Anatol > > > > *From:* Dmitry Stogov [mailto:dmitry@zend.com] > *Sent:* Tuesday, April 14, 2015 2:39 PM > *To:* Anatol Belski > *Cc:* php-cvs@lists.php.net > *Subject:* Re: [PHP-CVS] com php-src: Partial revert of > 1a7798c786466977ef8d2bc95b40a435c47c845a that introduced wrong memory > access: ext/fileinfo/libmagic/softmagic.c > > > > Hi Anatol, > > Please review. > I have no idea what did you do in that patch, but it was wrong. > > Thanks. Dmitry. > > > > On Tue, Apr 14, 2015 at 3:35 PM, Dmitry Stogov <dmitry@php.net> wrote: > > Commit: 753fa0c593aa9be8ca749b2dc8b8999f587f5784 > Author: Dmitry Stogov <dmitry@zend.com> Tue, 14 Apr 2015 > 15:35:33 +0300 > Parents: 5db1a56705d6f3f4f09daa8f922dcfbc36afbc0f > Branches: master > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=753fa0c593aa9be8ca749b2dc8b8999f587f5784 > > Log: > Partial revert of 1a7798c786466977ef8d2bc95b40a435c47c845a that introduced > wrong memory access > > Changed paths: > M ext/fileinfo/libmagic/softmagic.c > > > Diff: > diff --git a/ext/fileinfo/libmagic/softmagic.c > b/ext/fileinfo/libmagic/softmagic.c > index e08d167..3fd30ff 100644 > --- a/ext/fileinfo/libmagic/softmagic.c > +++ b/ext/fileinfo/libmagic/softmagic.c > @@ -624,7 +624,6 @@ mprint(struct magic_set *ms, struct magic *m) > t = ms->offset + sizeof(double); > break; > > - case FILE_SEARCH: > case FILE_REGEX: { > char *cp; > int rval; > @@ -648,6 +647,15 @@ mprint(struct magic_set *ms, struct magic *m) > break; > } > > + case FILE_SEARCH: > + if (file_printf(ms, F(ms, m, "%s"), m->value.s) == -1) > + return -1; > + if ((m->str_flags & REGEX_OFFSET_START)) > + t = ms->search.offset; > + else > + t = ms->search.offset + m->vallen; > + break; > + > case FILE_DEFAULT: > case FILE_CLEAR: > if (file_printf(ms, "%s", m->desc) == -1) > > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > > >