From patchwork Mon Feb 16 18:06:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 440310 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 7B64C1401DA for ; Tue, 17 Feb 2015 05:07:01 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; q=dns; s=default; b=m5p5f7/ag3pNE+oYKPNW7OMwOWZ6Y O9K8q/XRbpzhH7Km2pXeY235US/2FbOCuubWksbgTu6YYD78mrDkRPGvmiIl89b9 FkWkqCgMMr/L4jQcxnAjKmR5x2sWN3+xIm8dnnHzl2lp4uO/m4l5ZKJ4IIgfntz5 RDWbAOwli6qHM0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; s=default; bh=/p7rculhOfMmx4GnFI7oOp2asVQ=; b=j9d sdxvFG2k/L6ITq7ysTj9ZAPyOl3/wFy/PE9fOW81DSbzT3fPuwXSYhOwRDhP0O4d F8aXhiBagjZGMapdJvKtZaRE7W7WTEg9Vw352vB31kkdMM3mNTJTSQUdroMzOwz7 jT9laaxODcfW24f9frKC04o1tGfeZ9cR9SjwX5zg= Received: (qmail 16867 invoked by alias); 16 Feb 2015 18:06:55 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 16857 invoked by uid 89); 16 Feb 2015 18:06:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KAM_FROM_URIBL_PCCC, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mail-vc0-f176.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:from:date:message-id:subject :to:content-type; bh=qDTM/AAcHBs96PHljvWczLXSXNbdFMrnFX4F7U6xlyc=; b=lAhWZCkIK+897D6JZOtwhDB/0l1FBMAnLYewBRKttBb9ni9efYFcSWmFVjMt+KlVwi pssbMIZm9fOq07HJi27NSnhl2EkNDIfyA+WTzLt2ly1XRPViaXOEP+elgNqp6tc8HEI/ 7JZNvMV8UaVH1PkoF1yI4hbDjhnZTsrkCMl0CKbgKuz1CLF25BPc/UeLLXhIbpzjdDJ1 nghHXQVHZgHRpuBW6cjrOrFCEptUePbr6xmHPi4qgrbrRGIsJUx/YL72B4tfcWfl89fc QycnRAsTSKdDpoz+KsQ/pPDvUMJwuamI663SHhq48ebPJNwoqhrACuPLRUrcPPHaTgEt Es0Q== X-Gm-Message-State: ALoCoQn5XhHtK9qwi+h/kV3BGKjNvzte2BM/K417ktDmMVs6eP/sAP0fPISRhfVflKE0KUkiHbAY X-Received: by 10.221.41.8 with SMTP id ts8mr1042100vcb.75.1424110005518; Mon, 16 Feb 2015 10:06:45 -0800 (PST) MIME-Version: 1.0 From: Paul Pluzhnikov Date: Mon, 16 Feb 2015 10:06:14 -0800 Message-ID: Subject: [patch] Fix BZ#16374 -- don't use mmap for FILE buffers To: GLIBC Devel Greetings, I believe we've reached a consensus in https://sourceware.org/ml/libc-alpha/2015-02/msg00010.html -- using mmap to allocate FILE buffers is really not a good idea. Attached patch replaces mmap()s with calloc()s. Doing this revealed a few cleanup bugs: wide stream and input-only buffers were not cleaned up under some conditions. Tested on Linux/x86_64 with no new failures. 2015-02-16 Paul Pluzhnikov [BZ #16374] * NEWS: mention 16374 * libio/filedoalloc.c (_IO_file_doallocate, _IO_default_doallocate): Use calloc instead of ALLOC_BUF (_IO_setb, _IO_default_finish): Use free instead of FREE_BUF (_IO_default_finish): Likewise. Fix cleanup bugs. (libc_freeres_fn): Clean up wide stream buffers. * libio/libio.h (struct _IO_FILE_complete): Delete _freeres_size, add _freeres_wbuf * libio/libioP.h (ROUND_TO_PAGE, FREE_BUF, ALLOC_BUF, ALLOC_WBUF): Delete * libio/wfiledoalloc.c (_IO_wfile_doallocate): Use calloc instead of ALLOC_WBUF * libio/wgenops.c (_IO_wdefault_doallocate): Likewise (_IO_wsetb, _IO_wdefault_finish): Use free instead of FREE_BUF --- Paul Pluzhnikov diff --git a/NEWS b/NEWS index 781f7a7..64a842b 100644 --- a/NEWS +++ b/NEWS @@ -9,8 +9,8 @@ Version 2.22 * The following bugs are resolved with this release: - 4719, 15467, 15790, 16560, 17569, 17792, 17912, 17932, 17944, 17949, - 17964, 17965, 17967, 17969. + 4719, 15467, 15790, 16374, 16560, 17569, 17792, 17912, 17932, 17944, + 17949, 17964, 17965, 17967, 17969. Version 2.21 diff --git a/libio/filedoalloc.c b/libio/filedoalloc.c index 918a24a..4be1b83 100644 --- a/libio/filedoalloc.c +++ b/libio/filedoalloc.c @@ -125,7 +125,10 @@ _IO_file_doallocate (fp) size = st.st_blksize; #endif } - ALLOC_BUF (p, size, EOF); + + p = calloc (size, 1); + if (p == NULL) return EOF; + _IO_setb (fp, p, p + size, 1); return 1; } diff --git a/libio/genops.c b/libio/genops.c index 6612997..c787842 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -398,7 +398,7 @@ _IO_setb (f, b, eb, a) int a; { if (f->_IO_buf_base && !(f->_flags & _IO_USER_BUF)) - FREE_BUF (f->_IO_buf_base, _IO_blen (f)); + free (f->_IO_buf_base); f->_IO_buf_base = b; f->_IO_buf_end = eb; if (a) @@ -587,7 +587,9 @@ _IO_default_doallocate (fp) { char *buf; - ALLOC_BUF (buf, _IO_BUFSIZ, EOF); + buf = calloc (_IO_BUFSIZ, 1); + if (buf == NULL) return EOF; + _IO_setb (fp, buf, buf+_IO_BUFSIZ, 1); return 1; } @@ -687,7 +689,7 @@ _IO_default_finish (fp, dummy) struct _IO_marker *mark; if (fp->_IO_buf_base && !(fp->_flags & _IO_USER_BUF)) { - FREE_BUF (fp->_IO_buf_base, _IO_blen (fp)); + free (fp->_IO_buf_base); fp->_IO_buf_base = fp->_IO_buf_end = NULL; } @@ -950,8 +952,6 @@ _IO_unbuffer_write (void) for (fp = (_IO_FILE *) _IO_list_all; fp; fp = fp->_chain) { if (! (fp->_flags & _IO_UNBUFFERED) - && (! (fp->_flags & _IO_NO_WRITES) - || (fp->_flags & _IO_IS_APPENDING)) /* Iff stream is un-orientated, it wasn't used. */ && fp->_mode != 0) { @@ -967,14 +967,30 @@ _IO_unbuffer_write (void) __sched_yield (); #endif - if (! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) + if (!dealloc_buffers) { - fp->_flags |= _IO_USER_BUF; - - fp->_freeres_list = freeres_list; - freeres_list = fp; - fp->_freeres_buf = fp->_IO_buf_base; - fp->_freeres_size = _IO_blen (fp); + const bool need_freeres = !(fp->_flags & _IO_USER_BUF) + || (!(fp->_flags2 & _IO_FLAGS2_USER_WBUF) && fp->_mode > 0); + + if (need_freeres) + { + fp->_freeres_buf = fp->_freeres_wbuf = NULL; + fp->_freeres_list = freeres_list; + freeres_list = fp; + + if (!(fp->_flags & _IO_USER_BUF)) + { + fp->_flags |= _IO_USER_BUF; + fp->_freeres_buf = fp->_IO_buf_base; + } + + if (fp->_mode > 0 + && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF)) + { + fp->_flags2 |= _IO_FLAGS2_USER_WBUF; + fp->_freeres_wbuf = fp->_wide_data->_IO_buf_base; + } + } } _IO_SETBUF (fp, NULL, 0); @@ -998,7 +1014,8 @@ libc_freeres_fn (buffer_free) while (freeres_list != NULL) { - FREE_BUF (freeres_list->_freeres_buf, freeres_list->_freeres_size); + free (freeres_list->_freeres_buf); + free (freeres_list->_freeres_wbuf); freeres_list = freeres_list->_freeres_list; } diff --git a/libio/libio.h b/libio/libio.h index 9ff1fb0..fd139d3 100644 --- a/libio/libio.h +++ b/libio/libio.h @@ -297,17 +297,17 @@ struct _IO_FILE_complete struct _IO_wide_data *_wide_data; struct _IO_FILE *_freeres_list; void *_freeres_buf; - size_t _freeres_size; + void *_freeres_wbuf; # else void *__pad1; void *__pad2; void *__pad3; void *__pad4; - size_t __pad5; + void *__pad5; # endif int _mode; /* Make sure we don't get into trouble again. */ - char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (size_t)]; + char _unused2[15 * sizeof (int) - 5 * sizeof (void *)]; #endif }; diff --git a/libio/libioP.h b/libio/libioP.h index d8604ca..bee9eaa 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -746,45 +746,6 @@ extern _IO_off64_t _IO_seekpos_unlocked (_IO_FILE *, _IO_off64_t, int) # define ftruncate __ftruncate # endif -# define ROUND_TO_PAGE(_S) \ - (((_S) + EXEC_PAGESIZE - 1) & ~(EXEC_PAGESIZE - 1)) - -# define FREE_BUF(_B, _S) \ - munmap ((_B), ROUND_TO_PAGE (_S)) -# define ALLOC_BUF(_B, _S, _R) \ - do { \ - (_B) = (char *) mmap (0, ROUND_TO_PAGE (_S), \ - PROT_READ | PROT_WRITE, \ - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \ - if ((_B) == (char *) MAP_FAILED) \ - return (_R); \ - } while (0) -# define ALLOC_WBUF(_B, _S, _R) \ - do { \ - (_B) = (wchar_t *) mmap (0, ROUND_TO_PAGE (_S), \ - PROT_READ | PROT_WRITE, \ - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \ - if ((_B) == (wchar_t *) MAP_FAILED) \ - return (_R); \ - } while (0) - -#else /* _G_HAVE_MMAP */ - -# define FREE_BUF(_B, _S) \ - free(_B) -# define ALLOC_BUF(_B, _S, _R) \ - do { \ - (_B) = (char*)malloc(_S); \ - if ((_B) == NULL) \ - return (_R); \ - } while (0) -# define ALLOC_WBUF(_B, _S, _R) \ - do { \ - (_B) = (wchar_t *)malloc(_S); \ - if ((_B) == NULL) \ - return (_R); \ - } while (0) - #endif /* _G_HAVE_MMAP */ #ifndef OS_FSTAT diff --git a/libio/wfiledoalloc.c b/libio/wfiledoalloc.c index 12425fd..9c7fbf6 100644 --- a/libio/wfiledoalloc.c +++ b/libio/wfiledoalloc.c @@ -95,7 +95,10 @@ _IO_wfile_doallocate (fp) size = fp->_IO_buf_end - fp->_IO_buf_base; if ((fp->_flags & _IO_USER_BUF)) size = (size + sizeof (wchar_t) - 1) / sizeof (wchar_t); - ALLOC_WBUF (p, size * sizeof (wchar_t), EOF); + + p = calloc (size, sizeof (wchar_t)); + if (p == NULL) return EOF; + _IO_wsetb (fp, p, p + size, 1); return 1; } diff --git a/libio/wgenops.c b/libio/wgenops.c index 69f3b95..afebee5 100644 --- a/libio/wgenops.c +++ b/libio/wgenops.c @@ -111,7 +111,7 @@ _IO_wsetb (f, b, eb, a) int a; { if (f->_wide_data->_IO_buf_base && !(f->_flags2 & _IO_FLAGS2_USER_WBUF)) - FREE_BUF (f->_wide_data->_IO_buf_base, _IO_wblen (f) * sizeof (wchar_t)); + free (f->_wide_data->_IO_buf_base); f->_wide_data->_IO_buf_base = b; f->_wide_data->_IO_buf_end = eb; if (a) @@ -195,8 +195,7 @@ _IO_wdefault_finish (fp, dummy) struct _IO_marker *mark; if (fp->_wide_data->_IO_buf_base && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF)) { - FREE_BUF (fp->_wide_data->_IO_buf_base, - _IO_wblen (fp) * sizeof (wchar_t)); + free (fp->_wide_data->_IO_buf_base); fp->_wide_data->_IO_buf_base = fp->_wide_data->_IO_buf_end = NULL; } @@ -426,7 +425,9 @@ _IO_wdefault_doallocate (fp) { wchar_t *buf; - ALLOC_WBUF (buf, _IO_BUFSIZ, EOF); + buf = calloc (_IO_BUFSIZ, 1); + if (buf == NULL) return EOF; + _IO_wsetb (fp, buf, buf + _IO_BUFSIZ, 1); return 1; }