On Fri, Sep 6, 2024 at 1:02 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > On Thu, Sep 05, 2024 at 02:17:14PM +0200, Uros Bizjak wrote: > > Usage of pseudo-random functions requires inclusion of > > <linux/prandom.h> header instead of <linux/random.h>. > > > > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx> > > Cc: Eric Biggers <ebiggers@xxxxxxxxxx> > > Cc: "Theodore Y. Ts'o" <tytso@xxxxxxx> > > Cc: Jaegeuk Kim <jaegeuk@xxxxxxxxxx> > > Cc. linux-fscrypt@xxxxxxxxxxxxxxx > > --- > > fs/crypto/keyring.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c > > index 6681a71625f0..e2c10b3b960b 100644 > > --- a/fs/crypto/keyring.c > > +++ b/fs/crypto/keyring.c > > @@ -21,7 +21,7 @@ > > #include <asm/unaligned.h> > > #include <crypto/skcipher.h> > > #include <linux/key-type.h> > > -#include <linux/random.h> > > +#include <linux/prandom.h> > > #include <linux/seq_file.h> > > > > 1. linux-fscrypt wasn't actually Cc'ed on this patch, due to the typo of > "Cc." instead of "Cc:". Uh, thanks for noticing. > 2. Currently <linux/random.h> includes <linux/prandom.h>, so the issue described > in the commit message does not exist. I assume this in changing in a later > patch that was not sent to me. The commit message should be rephrased to > clarify that this change is needed because of header refactoring, as > otherwise it sounds like a bug fix. Yes, the goal of the patch series is to allow inclusion of <linux/percpu.h> in linux/prandom.h. The major roadblock to achieve this is the inclusion of <linux/prandom.h> in linux/random.h, since this creates circular dependency which doesn't allow the inclusion of <linux/percpu.h>. Please note that this legacy include is removed in patch 17. I will mention this in the 0000 commit. I will also mention header refactoring in individual commits. OTOH, I think that while particular maintainers are CC'd only on their individual patches, it is better to send the whole series to all mentioned mailing lists. Will do that in v2. > 3. The proposed change does not make sense, because fs/crypto/keyring.c does not > use any "pseudo-random functions". It does use get_random_once(), which is > defined in <linux/once.h>. Currently <linux/random.h> includes > <linux/prandom.h> which includes <linux/once.h>. If the inclusion of > prandom.h by random.h is going away, then perhaps random.h should include > once.h directly so that get_random_once() continues to work? If not, then > this file should include once.h. Either way it should not include prandom.h. Due to the tricky nature of the whole series I tried to make individual patches as mechanical as possible, IOW, change the inclusion of <linux/random.h> to <linux/prandom.h>. Looking at fs/crypto/keyring.c, it should still include <linux/random.h> but should also include <linux/once.h>. The latter was just accidentally included via <linux/random.h>/<linux/prandom.h> path which is going away. Will fix in v2. Thanks, Uros.
|