Mail archive
alpine-aports

Re: [alpine-aports] [PATCH] community/minidlna: patch for potential segfaults

From: Timo Teras <timo.teras_at_iki.fi>
Date: Mon, 6 Feb 2017 13:11:05 +0200

On Mon, 06 Feb 2017 11:52:13 +0100
Huszty Gergő <huszty.gergo_at_digitaltrip.hu> wrote:

> Aport list added to CC.
>
> 2017-02-06 11:26 időpontban Huszty Gergő ezt írta:
>
> > Please see my comments inline. So I didn't dig myself too much into
> > the details, but stack is exhausted always at this point on my
> > system and maybe other users suffer from the same issue.
> > I didn't want to touch the code too much and also didn't want to
> > increase the very typical 8192k stack ulimit size. I don't know how
> > typical is to set exclusive stack size for an app. Do you think that
> > would be a better solution?

The 8M ulimit based stack size affects only the main thread. The
program creates additional thread(s) in minidlna.c:1052. On those
threads the musl default is 80k. You should use
pthread_attr_setstacksize to change this.

For example, see:
http://git.alpinelinux.org/cgit/aports/tree/main/mpd/stacksize.patch

> > 2017-02-06 10:43 időpontban Timo Teras ezt írta:
> >
> > On Sat, 4 Feb 2017 20:13:54 +0000
> > Gergo Huszty <huszty.gergo_at_digitaltrip.hu> wrote:
> >
> > From: libesz <huszty.gergo_at_digitaltrip.hu>
> >
> > Nfo parsing related fixes added in a patch.
> > - uninitalized string (GetVideoMetadata() - nfo) -> memset to 0
> > - stack was kicked with 64k buffer unconditionally (parse_nfo() -
> > buf) -> now it is on heap and malloc'd size depends on filesize ---
> > community/minidlna/10-minidlna-nfo.patch | 42
> > ++++++++++++++++++++++++++++++++
> > community/minidlna/APKBUILD | 14 +++++++---- 2 files
> > changed, 51 insertions(+), 5 deletions(-) create mode 100644
> > community/minidlna/10-minidlna-nfo.patch
> >
> > diff --git a/community/minidlna/10-minidlna-nfo.patch
> > b/community/minidlna/10-minidlna-nfo.patch new file mode 100644
> > index 0000000..8e3497c
> > --- /dev/null
> > +++ b/community/minidlna/10-minidlna-nfo.patch
> > _at_@ -0,0 +1,42 @@
> > +--- a/metadata.c
> > ++++ b/metadata.c
> > +_at_@ -160,7 +160,7 @@
> > + parse_nfo(const char *path, metadata_t *m)
> > + {
> > + FILE *nfo;
> > +- char buf[65536];
> > ++ char *buf;
> > + struct NameValueParserData xml;
> > + struct stat file;
> > + size_t nread;
> > +_at_@ -172,11 +172,13 @@
> > + DPRINTF(E_INFO, L_METADATA, "Not parsing very
> > large .nfo file %s\n", path);
> > + return;
> > + }
> > ++ buf = 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 65536, so using
> > malloc does not help much here. Large files seem to be ignored
> > intentionally.
> >
> > Though, if this fixes issues it's probably due to reducing stack
> > usage. It might be better to increase default stack size, as large
> > stack buffers might be used elsewhere.
> Well that's interesting because the program fails every time exactly
> here.

Probably the inotify thread crashing due to out of stack.

> >> ++ memset(buf, '\0', file.st_size+1);
> >> + DPRINTF(E_DEBUG, L_METADATA, "Parsing .nfo file: %s\n",
> >> path);
> >> + nfo = fopen(path, "r");
> >> + if( !nfo )
> >> + return;
> >> +- nread = fread(&buf, 1, sizeof(buf), nfo);
> >> ++ nread = fread(buf, 1, file.st_size, nfo);
> >> +
> >> + ParseNameValue(buf, nread, &xml, 0);
> >> +
> >> +_at_@ -676,6 +679,7 @@
> >> +
> >> + memset(&m, '\0', sizeof(m));
> >> + memset(&video, '\0', sizeof(video));
> >> ++ memset(nfo, '\0', sizeof(nfo));
> >> +
> >> + //DEBUG DPRINTF(E_DEBUG, L_METADATA, "Parsing video
> >> %s...\n", name);
> >> + if ( stat(path, &file) != 0 )
> >
> > This probable does not fix anything. nfo is strcpy:ied to.
> Yes it is, but the string was not null terminated so it was passed
> forward as a filepath with junk at the end. At least I saw it in the
> debugger.

That is normal. Debugger shows the whole char array. When used the
first zero byte terminates the string, and it'd handled right.


---
Unsubscribe:  alpine-aports+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-aports+help_at_lists.alpinelinux.org
---
Received on Mon Feb 06 2017 - 13:11:05 GMT