From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 18720 invoked from network); 12 Oct 2020 15:02:33 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 12 Oct 2020 15:02:33 -0000 Received: (qmail 23795 invoked by uid 550); 12 Oct 2020 15:02:30 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 23767 invoked from network); 12 Oct 2020 15:02:29 -0000 Date: Mon, 12 Oct 2020 11:02:16 -0400 From: Rich Felker To: Jouni Roivas Cc: musl@lists.openwall.com Message-ID: <20201012150215.GH17637@brightrain.aerifal.cx> References: <20201012125901.2629590-1-jouni.roivas@tuxera.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201012125901.2629590-1-jouni.roivas@tuxera.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH] Skip writing first iovec if it's empty On Mon, Oct 12, 2020 at 03:59:01PM +0300, Jouni Roivas wrote: > In case the first iovec is empty, skip writing it. Usually writing > zero length iovec is no-op, but in case of certain special cases this > causes the write to fail. > > This affects at least cgroups under sysfs, since it doesn't properly > support writev with multiple iovec. For those kernel tends to handle > them as simple as possible, passing each iovec separately. In case > of zero length write into cgroups file causes kernel to return error. > Thus if writing the first iovec fails for being zero length, it > causes the whole write to fail even if writing the second iovec would > succeed. This happens for example when doing unbuffered write with > musl to a file under cgroups. Fix the issue here, since if kernel > gets fixed for this specific case, it still doesn't get fixed for > older kernels, nor any other possible similar case. > --- > src/stdio/__stdio_write.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/src/stdio/__stdio_write.c b/src/stdio/__stdio_write.c > index d2d89475..eedce03a 100644 > --- a/src/stdio/__stdio_write.c > +++ b/src/stdio/__stdio_write.c > @@ -11,6 +11,10 @@ size_t __stdio_write(FILE *f, const unsigned char *buf, size_t len) > size_t rem = iov[0].iov_len + iov[1].iov_len; > int iovcnt = 2; > ssize_t cnt; > + if (iov[0].iov_len == 0) { > + iov++; > + iovcnt--; > + } > for (;;) { > cnt = syscall(SYS_writev, f->fd, iov, iovcnt); > if (cnt == rem) { > -- > 2.25.1 I don't think this is sufficient to make writing procfs/sysfs files via stdio work reliably; their interface is fundamentally incompatible with flexibility of implementation in buffering. But it might be preferable to do this still for other reasons. If we go forward with this I think the commit message needs major rework so as not to be implying that it makes the procfs/sysfs thing into a supported usage case. Note that commit e7eeeb9f2a4a358fb0bbed81e145ef5538ff60f0 did the analog for __stdio_read in a way that's probably slightly better, but that would need adaptation to work for the write case. Rich