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

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Aug 19, 2025 at 10:19:41AM +0200, Marco Felsch wrote:
> 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.

That's your/our usecase, but you don't know what others are doing and a
"bootm foo.fit" should just work without complaining about a relative
path.

> 
> > 
> > > +
> > > +	/* 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.

canonicalize_path() returns an absolute path.

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 |