Re: [PATCH v5 09/11] FIT: fit_open: make filename handling more robust

[Marco Felsch <m.felsch@pengutronix.de>]

On 25-08-19, Sascha Hauer wrote:
> On Mon, Aug 18, 2025 at 07:26:17PM +0200, Marco Felsch wrote:
> > Require the filename to start at the root '/' directory and resolve any
> > possible link to make the filename handling more robust.
> > 
> > This is in preparation of adding cached fit_open support.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  common/image-fit.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 0067f46e60bc954b418aef3398e2c10856b41c02..1cb407d4d86cb3d0a643149bb08c46caadcd56fe 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -972,12 +972,25 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
> >   *
> >   * Return: A handle to a FIT image or a ERR_PTR
> >   */
> > -struct fit_handle *fit_open(const char *filename, bool verbose,
> > +struct fit_handle *fit_open(const char *_filename, bool verbose,
> >  			    enum bootm_verify verify, loff_t max_size)
> >  {
> >  	struct fit_handle *handle;
> > +	char *filename;
> >  	int ret;
> >  
> > +	if (*_filename != '/') {
> > +		pr_err("The FIT filename must start with '/'\n");
> > +		return ERR_PTR(-EINVAL);
> > +	}
> 
> Drop this. We want to be able to call fit_open() with relative pathes...

IMHO FIT containers are only used in verified-boot setups. I wouldn't
allow relative paths in such use-cases.

> 
> > +
> > +	/* dirfd is ignored, since _filename is absolute */
> > +	filename = canonicalize_path(AT_FDCWD, _filename);
> 
> ...and you are calling canonicalize_path() anyway which gives you an
> absolute path from the potentially relative input path.

But we don't know the base directory to which this path would relative
too.

> > +	if (!filename) {
> > +		pr_err("Failed to resolve %s with %s\n", _filename, strerror(errno));
> 
> 		pr_err("Cannot open %s: %m\n", filename);
> 
> I think the "Failed to resolve" is misleading here.

You're right, I will change this.

Regards,
  Marco

> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>