linux-kernel - Re: [PATCH 3/9] trinity: Add load/unload IDU files

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: <YuE6IrM/8slDjsdL@kroah.com>
Date:   Wed, 27 Jul 2022 15:14:10 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jiho Chu <jiho.chu@...sung.com>
Cc:     arnd@...db.de, linux-kernel@...r.kernel.org,
        yelini.jeong@...sung.com, myungjoo.ham@...sung.com
Subject: Re: [PATCH 3/9] trinity: Add load/unload IDU files

On Mon, Jul 25, 2022 at 03:53:02PM +0900, Jiho Chu wrote:
> +static int triv2_idu_load_file(struct trinity_driver *drv, const char *dirpath,
> +			       const char *file_name,
> +			       struct trinity_resv_mem *sector)
> +{
> +	struct device *dev = drv_to_dev_ptr(drv);
> +	struct trinity_resv_mem mem;
> +	char filepath[NAME_MAX];
> +	struct kstat *stat;
> +	struct file *filp;
> +	loff_t pos = 0;
> +	size_t size;
> +	int ret;
> +
> +	dev = drv_to_dev_ptr(drv);
> +	stat = vmalloc(sizeof(*stat));
> +	if (stat == NULL)
> +		return -ENOMEM;
> +
> +	/* if dirpath is null, use the default path */
> +	if (dirpath)
> +		snprintf(filepath, NAME_MAX, "%s/%s", dirpath, file_name);
> +	else
> +		snprintf(filepath, NAME_MAX, TRIV2_IDU_DIRPATH_FMT "/%s",
> +			 utsname()->release, file_name);
> +
> +	filp = filp_open(filepath, O_RDONLY, 0400);

That is cute.  And totally not ok.

Please never do this, that is not how to properly load a firmware blob
in the kernel.  This is racy and broken and probably a huge security
hole.

Heck, I wrote an article about this very topic, way back in 2005, with
the title of, "Things you should never do in the kernel" and can be seen
here:
	https://www.linuxjournal.com/article/8110

This should not be news to anyone, again, never do this.

thanks,

greg k-h