linux-kernel - Re: direct-io: squelch maybe-uninitialized warning in do_direct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Jul 2014 11:48:30 +0200
From:	Paul Bolle <pebolle@...cali.nl>
To:	Boaz Harrosh <openosd@...il.com>
Cc:	Christoph Hellwig <hch@...radead.org>, viro@...iv.linux.org.uk,
	Borislav Petkov <bp@...en8.de>,
	Sam Ravnborg <sam@...nborg.org>, pramod.gurav.etc@...il.com,
	Jason Cooper <jason@...edaemon.net>,
	Markus Mayer <markus.mayer@...aro.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: direct-io: squelch maybe-uninitialized warning in do_direct_IO()

Boaz,

On Thu, 2014-07-17 at 12:37 +0300, Boaz Harrosh wrote:
> From: Paul Bolle <pebolle@...cali.nl>
> 
> The following warnings:
> 
>   fs/direct-io.c: In function ‘__blockdev_direct_IO’:
>   fs/direct-io.c:1011:12: warning: ‘to’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   fs/direct-io.c:913:16: note: ‘to’ was declared here
>   fs/direct-io.c:1011:12: warning: ‘from’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   fs/direct-io.c:913:10: note: ‘from’ was declared here
> 
> are false positive because dio_get_page() either fails, or sets both
> 'from' and 'to'.
> 
> Maybe it's better to move initializing "to" and "from" out of
> dio_get_page(). That _might_ make it easier for both the the reader and
> the compiler to understand what's going on. Something like this:
> 
> Christoph Hellwig said ...
> The fix of moving the code defintively looks nicer, while I think
> uninitialized_var is horrible wart that won't get anywhere near my code.
> 
> Boaz Harrosh I agree with Christoph and Paul
> 
> Signed-off-by: Paul Bolle <pebolle@...cali.nl>

This is simply not coorect!

> Signed-off-by: Boaz Harrosh <boaz@...xistor.com>
> ---
>  fs/direct-io.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 98040ba..2f024fc 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -198,9 +198,8 @@ static inline int dio_refill_pages(struct dio *dio, struct dio_submit *sdio)
>   * L1 cache.
>   */
>  static inline struct page *dio_get_page(struct dio *dio,
> -		struct dio_submit *sdio, size_t *from, size_t *to)
> +		struct dio_submit *sdio)
>  {
> -	int n;
>  	if (dio_pages_present(sdio) == 0) {
>  		int ret;
>  
> @@ -209,10 +208,7 @@ static inline struct page *dio_get_page(struct dio *dio,
>  			return ERR_PTR(ret);
>  		BUG_ON(dio_pages_present(sdio) == 0);
>  	}
> -	n = sdio->head++;
> -	*from = n ? 0 : sdio->from;
> -	*to = (n == sdio->tail - 1) ? sdio->to : PAGE_SIZE;
> -	return dio->pages[n];
> +	return dio->pages[sdio->head];
>  }
>  
>  /**
> @@ -911,11 +907,15 @@ static int do_direct_IO(struct dio *dio, struct dio_submit *sdio,
>  	while (sdio->block_in_file < sdio->final_block_in_request) {
>  		struct page *page;
>  		size_t from, to;
> -		page = dio_get_page(dio, sdio, &from, &to);
> +
> +		page = dio_get_page(dio, sdio);
>  		if (IS_ERR(page)) {
>  			ret = PTR_ERR(page);
>  			goto out;
>  		}
> +		from = sdio->head ? 0 : sdio->from;
> +		to = (sdio->head == sdio->tail - 1) ? sdio->to : PAGE_SIZE;
> +		sdio->head++;
>  
>  		while (from < to) {
>  			unsigned this_chunk_bytes;	/* # of bytes mapped */


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/