linux-kernel - Re: A field in files_struct has been used without initialization

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]

Message-ID: <Y0B6+0MLZI/nv1aC@ZenIV>
Date:   Fri, 7 Oct 2022 20:16:11 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Abd-Alrhman Masalkhi <abd.masalkhi@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: A field in files_struct has been used without initialization

On Thu, Oct 06, 2022 at 12:57:28PM +0200, Abd-Alrhman Masalkhi wrote:
> > new_fdt->close_on_exec itself has not been initialized, is it intended
> > to be like this.
> 
> I meant:
> 
> newf->close_on_exec_init itself has not been initialized ...

Huh? close_on_exec_init is an array, and this assignment stores the address
of its first (and only) element into newf->fdtab.close_on_exec.  So it's
basically
	newf->fdtab.close_on_exec = &newf->close_on_exec_init[0];

->fdtab and ->close_on_exec_init are to be used only if we need no more than
BITS_PER_LONG descriptors.  It's common enough to make avoiding a separate
allocation (and separate cacheline on following the pointer chain) worth
the trouble.

Note that we do not use newf->fdtab directly - we use newf->fdt.

What happens here is
        new_fdt = &newf->fdtab;
	...
	set newf->fdtab contents for the case we need few descriptors

	if we need more
		allocate a separate struct fdtable, bitmaps, etc.
		set the contents of that separate fdtable
		new_fdt = that new fdtable

	copy bitmaps into whatever new_fdt->close_on_exec and
	new_fdt->open_fds are pointing at.

	copy file pointers into whatever new_fdt->fd[] points at.

	set newf->fdt to new_fdt.

The value of newf->close_on_exec_init is simply newf + known constant.
It needs no initialization at all.  The *contents* of the array
it points to is used only if new_fdt remains pointing to newf->fdtab;
in that case it's initialized by
	copy_fd_bitmaps(new_fdt, old_fdt, open_files);

IOW, for few-descriptors case we end up with

newf:
	fdt			points to newf->fdtab
	fdtab.close_on_exec	points to newf->close_on_exec_init[0]
	fdtab.full_fd_bits	points to newf->full_fd_bits_init[0]
	fdtab.open_fds		points to newf->open_fds_init[0]
	fdtab.fd		points to newf->fd_array[0]
	fdtab.max_fds		max capacity; will be BITS_PER_LONG
	close_on_exec_init[0]	hosts the close_on_exec bitmap
	full_fd_bits_init[0]	hosts the full_fd_bits bitmap
	open_fds_init[0]	hosts the open_fds bitmap
	fd_array[]		contains file pointers (BITS_PER_LONG of them)

For more-than-a-few-descriptors case we have

array of pointers (from first kvmalloc() in alloc_fdtable()): contains file pointers
array of unsigned long (from the second kmvalloc() there): hosts the bitmaps
fdtable (allocated in alloc_fdtable()):
	open_fds		points to the beginning of bitmap-hosting array
	close_on_exec		points to the middle third of the same array
	full_fd_bits		points to the last third of the same array
	fd			points to array of struct file pointers.
	max_fds			max capacity, matches the sizes of arrays.
newf:
	fdt			points to fdtable
	fdtab, ..._init and fd_array 	unused

Might be useful to take a piece of paper and draw the picture,
I really don't want to bother with ASCII graphics for that...