linux-kernel - Re: [PATCH 1/2] efi: Export boot-services code and data as debugfs-blobs

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 31 Mar 2018 16:10:30 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        Peter Jones <pjones@...hat.com>,
        Dave Olsthoorn <dave@...aar.me>, x86@...nel.org,
        linux-efi@...r.kernel.org
Subject: Re: [PATCH 1/2] efi: Export boot-services code and data as
 debugfs-blobs

On Sat, Mar 31, 2018 at 02:19:43PM +0200, Hans de Goede wrote:
> Sometimes it is useful to be able to dump the efi boot-services code and
> data. This commit adds these as debugfs-blobs to /sys/kernel/debug/efi,
> but only if efi=debug is passed on the kernel-commandline as this requires
> not freeing those memory-regions, which costs 20+ MB of RAM.
> 
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> ---
>  arch/x86/platform/efi/quirks.c |  4 +++
>  drivers/firmware/efi/efi.c     | 57 ++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 5b513ccffde4..0f968c7bcfec 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -374,6 +374,10 @@ void __init efi_free_boot_services(void)
>  	int num_entries = 0;
>  	void *new, *new_md;
>  
> +	/* Keep all regions for /sys/kernel/debug/efi */
> +	if (efi_enabled(EFI_DBG))
> +		return;
> +
>  	for_each_efi_memory_desc(md) {
>  		unsigned long long start = md->phys_addr;
>  		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index cd42f66a7c85..fddc5f706fd2 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -18,6 +18,7 @@
>  #include <linux/kobject.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> +#include <linux/debugfs.h>
>  #include <linux/device.h>
>  #include <linux/efi.h>
>  #include <linux/of.h>
> @@ -316,6 +317,59 @@ static __init int efivar_ssdt_load(void)
>  static inline int efivar_ssdt_load(void) { return 0; }
>  #endif
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +#define EFI_DEBUGFS_MAX_BLOBS 32
> +
> +struct debugfs_blob_wrapper debugfs_blob[EFI_DEBUGFS_MAX_BLOBS];
> +
> +static void __init efi_debugfs_init(void)
> +{
> +	struct dentry *efi_debugfs;
> +	efi_memory_desc_t *md;
> +	char name[32];
> +	int type_count[EFI_BOOT_SERVICES_DATA + 1] = {};
> +	int i = 0;
> +
> +	efi_debugfs = debugfs_create_dir("efi", NULL);
> +	if (IS_ERR_OR_NULL(efi_debugfs)) {
> +		pr_warn("Could not create efi debugfs entry\n");
> +		return;
> +	}

{sigh}

No, don't warn, or complain, or do anything else if a debugfs call
fails.  Just keep on moving, you can always use the return value
properly in any future call if you need it, and no code flow should ever
care if a debugfs call succeeded or failed.


>  /*
>   * We register the efi subsystem with the firmware subsystem and the
>   * efivars subsystem with the efi subsystem, if the system was booted with
> @@ -360,6 +414,9 @@ static int __init efisubsys_init(void)
>  		goto err_remove_group;
>  	}
>  
> +	if (efi_enabled(EFI_DBG))
> +		efi_debugfs_init();

You never remove the directory?

thanks,

greg k-h