X-Original-To: alpine-aports@lists.alpinelinux.org Received: from digitaltrip.hu (digitaltrip.hu [87.229.24.37]) by lists.alpinelinux.org (Postfix) with ESMTP id 27CBE5C424E for ; Mon, 6 Feb 2017 10:52:16 +0000 (GMT) Received: from localhost (localhost [127.0.0.1]) by digitaltrip.hu (Postfix) with ESMTP id 8A675C035C; Mon, 6 Feb 2017 11:52:14 +0100 (CET) Received: from digitaltrip.hu ([127.0.0.1]) by localhost (digitaltrip.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZV32KJawGPCR; Mon, 6 Feb 2017 11:52:13 +0100 (CET) Received: from digitaltrip.hu (localhost [127.0.0.1]) by digitaltrip.hu (Postfix) with ESMTP id 27828C0356; Mon, 6 Feb 2017 11:52:13 +0100 (CET) X-Mailinglist: alpine-aports Precedence: list List-Id: Alpine Development List-Unsubscribe: List-Post: List-Help: List-Subscribe: MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="=_cefde382ddf3146540c89c9a171d8c0f" Date: Mon, 06 Feb 2017 11:52:13 +0100 From: =?UTF-8?Q?Huszty_Gerg=C5=91?= To: Timo Teras Cc: alpine-aports@lists.alpinelinux.org Subject: Re: [alpine-aports] [PATCH] community/minidlna: patch for potential segfaults Reply-To: huszty.gergo@digitaltrip.hu Mail-Reply-To: huszty.gergo@digitaltrip.hu In-Reply-To: References: <20170204201354.26019-1-huszty.gergo@digitaltrip.hu> <20170206114306.4920c40c@vostro.util.wtbts.net> Message-ID: X-Sender: huszty.gergo@digitaltrip.hu User-Agent: Roundcube Webmail/1.2.0 --=_cefde382ddf3146540c89c9a171d8c0f Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8" QXBvcnQgbGlzdCBhZGRlZCB0byBDQy4gDQoNCjIwMTctMDItMDYgMTE6MjYgaWTFkXBvbnRiYW4g SHVzenR5IEdlcmfFkSBlenQgw61ydGE6DQoNCj4gSGksDQo+IFBsZWFzZSBzZWUgbXkgY29tbWVu dHMgaW5saW5lLiBTbyBJIGRpZG4ndCBkaWcgbXlzZWxmIHRvbyBtdWNoIGludG8NCj4gdGhlIGRl dGFpbHMsIGJ1dCBzdGFjayBpcyBleGhhdXN0ZWQgYWx3YXlzIGF0IHRoaXMgcG9pbnQgb24gbXkg c3lzdGVtDQo+IGFuZCBtYXliZSBvdGhlciB1c2VycyBzdWZmZXIgZnJvbSB0aGUgc2FtZSBpc3N1 ZS4NCj4gSSBkaWRuJ3Qgd2FudCB0byB0b3VjaCB0aGUgY29kZSB0b28gbXVjaCBhbmQgYWxzbyBk aWRuJ3Qgd2FudCB0bw0KPiBpbmNyZWFzZSB0aGUgdmVyeSB0eXBpY2FsIDgxOTJrIHN0YWNrIHVs aW1pdCBzaXplLiBJIGRvbid0IGtub3cgaG93DQo+IHR5cGljYWwgaXMgdG8gc2V0IGV4Y2x1c2l2 ZSBzdGFjayBzaXplIGZvciBhbiBhcHAuIERvIHlvdSB0aGluayB0aGF0DQo+IHdvdWxkIGJlIGEg YmV0dGVyIHNvbHV0aW9uPw0KPiANCj4gVGhhbmtzLA0KPiBHZXJnbw0KPiANCj4gMjAxNy0wMi0w NiAxMDo0MyBpZMWRcG9udGJhbiBUaW1vIFRlcmFzIGV6dCDDrXJ0YToNCj4gDQo+IE9uIFNhdCwg IDQgRmViIDIwMTcgMjA6MTM6NTQgKzAwMDANCj4gR2VyZ28gSHVzenR5IDxodXN6dHkuZ2VyZ29A ZGlnaXRhbHRyaXAuaHU+IHdyb3RlOg0KPiANCj4gRnJvbTogbGliZXN6IDxodXN6dHkuZ2VyZ29A ZGlnaXRhbHRyaXAuaHU+DQo+IA0KPiBOZm8gcGFyc2luZyByZWxhdGVkIGZpeGVzIGFkZGVkIGlu IGEgcGF0Y2guDQo+IC0gdW5pbml0YWxpemVkIHN0cmluZyAoR2V0VmlkZW9NZXRhZGF0YSgpIC0g bmZvKSAtPiBtZW1zZXQgdG8gMA0KPiAtIHN0YWNrIHdhcyBraWNrZWQgd2l0aCA2NGsgYnVmZmVy IHVuY29uZGl0aW9uYWxseSAocGFyc2VfbmZvKCkgLQ0KPiBidWYpIC0+IG5vdyBpdCBpcyBvbiBo ZWFwIGFuZCBtYWxsb2MnZCBzaXplIGRlcGVuZHMgb24gZmlsZXNpemUgLS0tDQo+IGNvbW11bml0 eS9taW5pZGxuYS8xMC1taW5pZGxuYS1uZm8ucGF0Y2ggfCA0Mg0KPiArKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKw0KPiBjb21tdW5pdHkvbWluaWRsbmEvQVBLQlVJTEQgICAgICAgICAg ICAgIHwgMTQgKysrKysrKy0tLS0gMiBmaWxlcw0KPiBjaGFuZ2VkLCA1MSBpbnNlcnRpb25zKCsp LCA1IGRlbGV0aW9ucygtKSBjcmVhdGUgbW9kZSAxMDA2NDQNCj4gY29tbXVuaXR5L21pbmlkbG5h LzEwLW1pbmlkbG5hLW5mby5wYXRjaA0KPiANCj4gZGlmZiAtLWdpdCBhL2NvbW11bml0eS9taW5p ZGxuYS8xMC1taW5pZGxuYS1uZm8ucGF0Y2gNCj4gYi9jb21tdW5pdHkvbWluaWRsbmEvMTAtbWlu aWRsbmEtbmZvLnBhdGNoIG5ldyBmaWxlIG1vZGUgMTAwNjQ0DQo+IGluZGV4IDAwMDAwMDAuLjhl MzQ5N2MNCj4gLS0tIC9kZXYvbnVsbA0KPiArKysgYi9jb21tdW5pdHkvbWluaWRsbmEvMTAtbWlu aWRsbmEtbmZvLnBhdGNoDQo+IEBAIC0wLDAgKzEsNDIgQEANCj4gKy0tLSBhL21ldGFkYXRhLmMN Cj4gKysrKyBiL21ldGFkYXRhLmMNCj4gK0BAIC0xNjAsNyArMTYwLDcgQEANCj4gKyBwYXJzZV9u Zm8oY29uc3QgY2hhciAqcGF0aCwgbWV0YWRhdGFfdCAqbSkNCj4gKyB7DQo+ICsgICAgIEZJTEUg Km5mbzsNCj4gKy0gICAgY2hhciBidWZbNjU1MzZdOw0KPiArKyAgICBjaGFyICpidWY7DQo+ICsg ICAgIHN0cnVjdCBOYW1lVmFsdWVQYXJzZXJEYXRhIHhtbDsNCj4gKyAgICAgc3RydWN0IHN0YXQg ZmlsZTsNCj4gKyAgICAgc2l6ZV90IG5yZWFkOw0KPiArQEAgLTE3MiwxMSArMTcyLDEzIEBADQo+ ICsgICAgICAgICBEUFJJTlRGKEVfSU5GTywgTF9NRVRBREFUQSwgIk5vdCBwYXJzaW5nIHZlcnkN Cj4gbGFyZ2UgLm5mbyBmaWxlICVzXG4iLCBwYXRoKTsNCj4gKyAgICAgICAgIHJldHVybjsNCj4g KyAgICAgfQ0KPiArKyAgICBidWYgPSBtYWxsb2MoZmlsZS5zdF9zaXplKzEpOyANCj4gVGhlIHJl dHVybiB2YWx1ZSBzaG91bGQgYmUgY2hlY2tlZCBmb3Igb3V0LW9mLW1lbW9yeS4NCiBZZXAsIGdv aW5nIHRvIGFkZCBpdC4gDQoNCj4gQWRkaXRpb25hbGx5IHRoZSBjb2RlIGFib3ZlIGNoZWNrcyBm b3IgbWF4aW11bSBvZiA2NTUzNiwgc28gdXNpbmcNCj4gbWFsbG9jIGRvZXMgbm90IGhlbHAgbXVj aCBoZXJlLiBMYXJnZSBmaWxlcyBzZWVtIHRvIGJlIGlnbm9yZWQNCj4gaW50ZW50aW9uYWxseS4N Cj4gDQo+IFRob3VnaCwgaWYgdGhpcyBmaXhlcyBpc3N1ZXMgaXQncyBwcm9iYWJseSBkdWUgdG8g cmVkdWNpbmcgc3RhY2sgdXNhZ2UuDQo+IEl0IG1pZ2h0IGJlIGJldHRlciB0byBpbmNyZWFzZSBk ZWZhdWx0IHN0YWNrIHNpemUsIGFzIGxhcmdlIHN0YWNrDQo+IGJ1ZmZlcnMgbWlnaHQgYmUgdXNl ZCBlbHNld2hlcmUuDQogV2VsbCB0aGF0J3MgaW50ZXJlc3RpbmcgYmVjYXVzZSB0aGUgcHJvZ3Jh bSBmYWlscyBldmVyeSB0aW1lIGV4YWN0bHkNCmhlcmUuIA0KDQo+PiArKyAgICBtZW1zZXQoYnVm LCAnXDAnLCBmaWxlLnN0X3NpemUrMSk7DQo+PiArICAgICBEUFJJTlRGKEVfREVCVUcsIExfTUVU QURBVEEsICJQYXJzaW5nIC5uZm8gZmlsZTogJXNcbiIsDQo+PiBwYXRoKTsNCj4+ICsgICAgIG5m byA9IGZvcGVuKHBhdGgsICJyIik7DQo+PiArICAgICBpZiggIW5mbyApDQo+PiArICAgICAgICAg cmV0dXJuOw0KPj4gKy0gICAgbnJlYWQgPSBmcmVhZCgmYnVmLCAxLCBzaXplb2YoYnVmKSwgbmZv KTsNCj4+ICsrICAgIG5yZWFkID0gZnJlYWQoYnVmLCAxLCBmaWxlLnN0X3NpemUsIG5mbyk7DQo+ PiArDQo+PiArICAgICBQYXJzZU5hbWVWYWx1ZShidWYsIG5yZWFkLCAmeG1sLCAwKTsNCj4+ICsN Cj4+ICtAQCAtNjc2LDYgKzY3OSw3IEBADQo+PiArDQo+PiArICAgICBtZW1zZXQoJm0sICdcMCcs IHNpemVvZihtKSk7DQo+PiArICAgICBtZW1zZXQoJnZpZGVvLCAnXDAnLCBzaXplb2YodmlkZW8p KTsNCj4+ICsrICAgIG1lbXNldChuZm8sICdcMCcsIHNpemVvZihuZm8pKTsNCj4+ICsNCj4+ICsg ICAgIC8vREVCVUcgRFBSSU5URihFX0RFQlVHLCBMX01FVEFEQVRBLCAiUGFyc2luZyB2aWRlbw0K Pj4gJXMuLi5cbiIsIG5hbWUpOw0KPj4gKyAgICAgaWYgKCBzdGF0KHBhdGgsICZmaWxlKSAhPSAw ICkNCj4gDQo+IFRoaXMgcHJvYmFibGUgZG9lcyBub3QgZml4IGFueXRoaW5nLiBuZm8gaXMgc3Ry Y3B5OmllZCB0by4NCiBZZXMgaXQgaXMsIGJ1dCB0aGUgc3RyaW5nIHdhcyBub3QgbnVsbCB0ZXJt aW5hdGVkIHNvIGl0IHdhcyBwYXNzZWQNCmZvcndhcmQgYXMgYSBmaWxlcGF0aCB3aXRoIGp1bmsg YXQgdGhlIGVuZC4gQXQgbGVhc3QgSSBzYXcgaXQgaW4gdGhlDQpkZWJ1Z2dlci4gDQoNCj4+IEBA IC02MywxMCArNjQsMTMgQEAgcGFja2FnZSgpIHsNCj4+IA0KPj4gbWQ1c3Vtcz0iMTk3MGU1NTNh MWViOGEzZTdlMzAyZTJjZTI5MmNiYzQgIG1pbmlkbG5hLTEuMS41LnRhci5neg0KPj4gNmRkMWVj NTU2MGFjMzBkN2EwNDI0NDEwMWU5MTJkNDUgIG1pbmlkbG5hLmluaXRkDQo+PiAtNTlkMTRjMWJm M2NkNjM3MTM4YmZhNThkYjcyNTVkNzggIG1pbmlkbG5hLmNvbmZkIg0KPj4gKzU5ZDE0YzFiZjNj ZDYzNzEzOGJmYTU4ZGI3MjU1ZDc4ICBtaW5pZGxuYS5jb25mZA0KPj4gK2RkYjJhNDE0MjYxMTA5 NTA5YTgxZTJlZGUwM2UxMmJhICAxMC1taW5pZGxuYS1uZm8ucGF0Y2giDQo+PiBzaGEyNTZzdW1z PSI4NDc3YWQwNDE2YmIyYWY1Y2Q4ZGE2ZGRlNmMwN2ZmZTFhNDEzNDkyYjdmZTQwYTM2MmJjODU4 N2JlMTVhYjliDQo+IA0KPiBDaGVja3N1bSBkaWQgbm90IG1hdGNoLiBUaGVyZSB3YXMgY2hhbmdl cyB0byBpdCBhZnRlciBsYXN0ICdhYnVpbGQNCj4gY2hlY2tzdW0nID8NCiBPbmx5IHR3ZWFrZWQg dGhlIGZpbGUgcGF0aHMgYSBiaXQgaW4gdGhlIGRpZmYu --=_cefde382ddf3146540c89c9a171d8c0f Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=UTF-8

Aport list added to CC.

2017-02-06 11:26 id=C5=91pontban Huszty Gerg=C5=91 ezt írta:

= Hi,
 Please see my comments inline.&nbs= p;So I didn't dig myself too much into
the details, but&= nbsp;stack is exhausted always at this point&= nbsp;on my system
I di= dn't want to touch the code too much&nbs= p;and also didn't want to
increase the very typical 8192k&nbs= p;stack ulimit size. I don't know how<= br />typical is to set&= nbsp;exclusive stack size for an app. Do = ;you think that
= would be a better solution?

Thanks,
 Gergo

On Sat, = ; 4 Feb 2017 20:13:54 +0000
Gergo Huszty <huszty.gergo@digitaltrip.hu> wrote:

From: libesz=  <huszty.gergo@digit= altrip.hu>

Nf= o parsing related fixes added in a patch= =2E
- uninitalized&nb= sp;string (GetVideoMetadata() - nfo) -> memset&= nbsp;to 0
- stac= k was kicked with 64k buffer unconditionally&= nbsp;(parse_nfo() -
b= uf) -> now it is on heap and mal= loc'd size depends on filesize ---
community/minidlna/10-minidlna-nfo.patch&= nbsp;| 42
+++++++++++= +++++++++++++++++++++
comm= unity/minidlna/APKBUILD        &nbs= p;     | 14 +++++++---- 2 file= s
changed, 51 in= sertions(+), 5 deletions(-) create mode 100644
community/minidlna/10-minidln= a-nfo.patch

diff&nbs= p;--git a/community/minidlna/10-minidlna-nfo.patch
b/community/minidlna/10-minidlna-nfo.patch&nb= sp;new file mode 100644
index 0000000..8e3497c
--- /dev/null
+++ b/community/minidlna/10-minidlna-nfo.patch
@@ -0,0 +1,42 @@+--- a/metadata.c
= ++++ b/metadata.c
+@@ -160,7 +160,7 @@
+ parse_nfo(const cha= r *path, metadata_t *m)
+ {
+&nb= sp;    FILE *nfo;
+-    char buf[65536];
++    char = *buf;
+   &= nbsp; struct NameValueParserData xml;
+     struct stat&= nbsp;file;
+  &n= bsp;  size_t nread;
+@@ -172,11 +172,13 @@
+         = ;DPRINTF(E_INFO, L_METADATA, "Not parsing verylarge .nfo file %s\= n", path);
+ &nb= sp;       return;
+     }
++    buf =3D&nb= sp;malloc(file.st_size+1);

The return value = should be checked for out-of-memory.

=
Yep, going to add = it.
Additionally = ;the code above checks for maximum of 65= 536, so using
ma= lloc does not help much here. Large file= s seem to be ignored
intentionally.

Though, if this fixes issues it's p= robably due to reducing stack usage.
<= span style=3D"white-space: nowrap;">It might be better = to increase default stack size, as large = ;stack
buffers might&= nbsp;be used elsewhere.

Well that's interesting because the program fails every time exactly here= =2E
++  &nb= sp; memset(buf, '\0', file.st_size+1);
+     DPRINTF(E_DEBUG,=  L_METADATA, "Parsing .nfo file: %s\n",
path);
+     nfo =3D fopen(= path, "r");
+ &n= bsp;   if( !nfo )
+         ret= urn;
+-   &= nbsp;nread =3D fread(&buf, 1, sizeof(buf), nfo= );
++   &nb= sp;nread =3D fread(buf, 1, file.st_size, nfo);
+
+     ParseNameValue(buf,&nb= sp;nread, &xml, 0);
+
+@@ -676,6&= nbsp;+679,7 @@
+
+     = ;memset(&m, '\0', sizeof(m));
+     memset(&video, '\= 0', sizeof(video));
+= +    memset(nfo, '\0', sizeof(nfo));+
+     //DEBUG DPRINTF(E_DEBU= G, L_METADATA, "Parsing video
%s...\n", name);
+     if ( stat(path,&nbs= p;&file) !=3D 0 )

This probable does&nbs= p;not fix anything. nfo is strcpy:ied to.
Yes it is, but the=  string was not null terminated so it&nb= sp;was passed
forward=  as a filepath with junk at the end= =2E At least I saw it in the
debugger.

@@ -63,10&nb= sp;+64,13 @@ package() {

md5sums=3D"1970e553a1eb8a3e7e302e2ce292cbc4  = minidlna-1.1.5.tar.gz
6dd1= ec5560ac30d7a04244101e912d45  minidlna.initd
-59d14c1bf3cd637138bfa58db7255d78  m= inidlna.confd"
+59d14c1bf3= cd637138bfa58db7255d78  minidlna.confd
+ddb2a414261109509a81e2ede03e12ba  10-mini= dlna-nfo.patch"
sha256sums= =3D"8477ad0416bb2af5cd8da6dde6c07ffe1a413492b7fe40a362bc8587be15ab9b=

Checksum did not = match. There was changes to it after las= t 'abuild
checksum'&n= bsp;?
Only tweaked the file&n= bsp;paths a bit in the diff.


--=_cefde382ddf3146540c89c9a171d8c0f-- --- Unsubscribe: alpine-aports+unsubscribe@lists.alpinelinux.org Help: alpine-aports+help@lists.alpinelinux.org ---