From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11982 Path: news.gmane.org!.POSTED!not-for-mail From: Szabolcs Nagy Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] stdio: implement fopencookie(3) Date: Thu, 5 Oct 2017 12:10:36 +0200 Message-ID: <20171005101036.GO15263@port70.net> References: <20171005064824.27894-1-nenolod@dereferenced.org> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1507198252 19443 195.159.176.226 (5 Oct 2017 10:10:52 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 5 Oct 2017 10:10:52 +0000 (UTC) User-Agent: Mutt/1.6.0 (2016-04-01) Cc: William Pitcock To: musl@lists.openwall.com Original-X-From: musl-return-11995-gllmg-musl=m.gmane.org@lists.openwall.com Thu Oct 05 12:10:48 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1e0377-0004OI-Qa for gllmg-musl@m.gmane.org; Thu, 05 Oct 2017 12:10:45 +0200 Original-Received: (qmail 26276 invoked by uid 550); 5 Oct 2017 10:10:50 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 26251 invoked from network); 5 Oct 2017 10:10:49 -0000 Mail-Followup-To: musl@lists.openwall.com, William Pitcock Content-Disposition: inline In-Reply-To: <20171005064824.27894-1-nenolod@dereferenced.org> Xref: news.gmane.org gmane.linux.lib.musl.general:11982 Archived-At: * William Pitcock [2017-10-05 06:48:24 +0000]: > +FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t iofuncs) > +{ > + FILE *f; > + struct winsize wsz; > + struct fcookie *fc; > + > + /* Check for valid initial mode character */ > + if (!strchr("rwa", *mode)) { > + errno = EINVAL; > + return 0; > + } > + > + /* Allocate FILE+fcookie+buffer or fail */ > + if (!(f=malloc(sizeof *f + sizeof *fc + UNGET + BUFSIZ))) return 0; > + > + /* Zero-fill only the struct, not the buffer */ > + memset(f, 0, sizeof *f); > + > + /* Impose mode restrictions */ > + if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD; > + > + /* Set up our fcookie */ > + fc = (void *)(f + 1); note that such malloc and pointer computation can be problematic if *fc has larger alignment requirement than *f > + fc->cookie = cookie; > + fc->iofuncs.read = iofuncs.read; > + fc->iofuncs.write = iofuncs.write; > + fc->iofuncs.seek = iofuncs.seek; > + fc->iofuncs.close = iofuncs.close; > + > + f->fd = -1; > + f->cookie = fc; > + f->buf = (unsigned char *)f + sizeof *f + sizeof *fc + UNGET; > + f->buf_size = BUFSIZ; > + f->lbf = EOF; > + > + /* Initialize op ptrs. No problem if some are unneeded. */ > + f->read = cookieread; > + f->write = cookiewrite; > + f->seek = cookieseek; > + f->close = cookieclose; > + > + if (!libc.threaded) f->lock = -1; > + i think this should be unconditional f->lock=0; if a function uses user callbacks, then it cannot be assumed that the process remains single-threaded during the lifetime of that function call. e.g. with your patch putc can be entered without locking, then the user write callback may create a thread that eventually also accesses f before putc finishes, introducing a data race. this shows why fopencookie hasnt been implemented yet: there is no specification what the callbacks may do, creating a thread is just one example that messes up the current stdio assumptions, recursively calling an stdio function on the same file from a callback may cause deadlock and it's not clear what should happen. we can leave the corner-cases unspecified, but then user code may start to depend on implementation internals that we cannot change later. (e.g. recent case in glibc: gnu make used a callback api for glob, but assumed one of the callbacks are not used by the implementation so it was not initialized, now glibc started using that callback and it took an elaborate api versioning scheme to avoid breaking old binaries and old make code compiled against new glibc. similar situation can arise with fopencookie if one assumes seek is not called in certain situations in glibc, but then it turns out musl does call it.) > + /* Add new FILE to open file list */ > + return __ofl_add(f); > +} > -- > 2.13.3