Re: [PATCH v5 4/8] ima: kexec: define functions to copy IMA log at soft boot

From: Mimi Zohar <zohar@linux.ibm.com>
To: Tushar Sugandhi <tusharsu@linux.microsoft.com>,
	Stefan Berger <stefanb@linux.ibm.com>,
	roberto.sassu@huaweicloud.com, roberto.sassu@huawei.com,
	eric.snowberg@oracle.com, ebiederm@xmission.com, noodles@fb.com,
	bauermann@kolabnow.com, linux-integrity@vger.kernel.org,
	kexec@lists.infradead.org
Cc: code@tyhicks.com, nramas@linux.microsoft.com, paul@paul-moore.com
Subject: Re: [PATCH v5 4/8] ima: kexec: define functions to copy IMA log at soft boot
Date: Wed, 21 Feb 2024 16:52:14 -0500	[thread overview]
Message-ID: <c19a84a7bf2f8313540eacdb2047ca5083dc8c1f.camel@linux.ibm.com> (raw)
In-Reply-To: <6c7158ef-2b12-4ba4-afc5-7618357d3aa4@linux.microsoft.com>

On Wed, 2024-02-14 at 22:55 -0800, Tushar Sugandhi wrote:
> 
> On 2/14/24 12:47, Stefan Berger wrote:
> > 
> > On 2/14/24 10:38, Tushar Sugandhi wrote:
> ...
> <snip/>
> ...
> > > +void kimage_file_post_load(struct kimage *image)
> > > +{
> > > +    ima_kexec_post_load(image);
> > > +}
> > > +
> > 
> > We get this here at this point but it disappears later -- missing header?
> > 
> > kernel/kexec_file.c:189:6: warning: no previous prototype for 
> > ‘kimage_file_post_load’ [-Wmissing-prototypes]
> >    189 | void kimage_file_post_load(struct kimage *image)
> >        |      ^~~~~~~~~~~~~~~~~~~~~
> > 
> > 
> Thanks Stefan.
> I was also getting it. 
> But couldn't figure out why. And I was puzzled why it was going away.
> 
> Since kimage_file_post_load() is called from the same file in patch 5/8,
> I don't see a need of declaring it in a header file like 
> include/linux/kexec.h.

Before trying to "fix" it, is there a need for a wrapper around
ima_kexec_post_load().  ima_add_kexec_buffer() is called directly.

Mimi

> 
> Making kimage_file_post_load() local static resolves the warning.
> But then it throws "defined but not used" warning. I will have to call 
> it from kexec_file_load syscall in this patch (4/8) instead 5/8 to 
> resolve that warning.
> 
> I will make the function a stub function in this patch and
> make it call ima_kexec_post_load(image) in the next patch to avoid any 
> potential bisect safe issues.
> 
> It aligns with the goals of patch 4/8 and 5/8 anyways.
> 
> +static void kimage_file_post_load(struct kimage *image)
> +{
> +	/*
> +	 * this will call ima_kexec_post_load(image) to map the segment
> +	 * and register the reboot notifier for moving the IMA log at
> +	 * kexec execute
> +	 */
> +}
> +
> 
> 
> @@ -410,6 +410,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, 
> int, initrd_fd,
>          kimage_terminate(image);
> +    if (!(flags & KEXEC_FILE_ON_CRASH))
> +        kimage_file_post_load(image);
> +
> 
> ...
> ...<snip/>
> ...
> 
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Thanks for the tag. I will apply it on the next version.
> 
> ~Tushar