Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <842fd97b-c958-7b0d-2c77-6927c7ab4d72@rasmusvillemoes.dk>
Date:   Wed, 23 Nov 2022 09:51:04 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v6 2/3] random: introduce generic vDSO getrandom()
 implementation
Content-Language: en-US, da
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        tglx@linutronix.de
Cc:     linux-crypto@vger.kernel.org, x86@kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
        Carlos O'Donell <carlos@redhat.com>,
        Linux API <linux-api@vger.kernel.org>
References: <20221121152909.3414096-1-Jason@zx2c4.com>
 <20221121152909.3414096-3-Jason@zx2c4.com>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
In-Reply-To: <20221121152909.3414096-3-Jason@zx2c4.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/11/2022 16.29, Jason A. Donenfeld wrote:

Cc += linux-api

> 
>       if (!new_block)
>         goto out;
>       new_cap = grnd_allocator.cap + num;
>       new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states));
>       if (!new_states) {
>         munmap(new_block, num * size_per_each);

Hm. This does leak an implementation detail of vgetrandom_alloc(),
namely that it is based on mmap() of that size rounded up to page size.
Do we want to commit to this being the proper way of disposing of a
succesful vgetrandom_alloc(), or should there also be a
vgetrandom_free(void *states, long num, long size_per_each)?

And if so, what color should the bikeshed really have. I.e.,

- does it need to take that size_per_each parameter which the kernel knows

- should it rather take the product so it can for now be a simple alias
for munmap

- should it also have a flags argument just because that's what all
well-behaving syscalls have these days...

Also, should vgetrandom_alloc() take a void *hint argument that
would/could be passed through to mmap() to give userspace some control
over where the memory is located - possibly only in the future, i.e.
insist on it being NULL for now, but it could open the possibility for
adding e.g. VGRND_MAP_FIXED[_NOREPLACE] that would translate to the
corresponding MAP_ flags.

Rasmus