[PATCH] tools: open FIT image for reading as O_RDONLY instead of O_RDWR

[PATCH] tools: open FIT image for reading as O_RDONLY instead of O_RDWR

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

The only usage of the fd is for the single read() below;
this prevented
	mkimage -f auto -A arm64 \
		-T kernel -C lz4 -d Image-6.6.15.lz4 \
		-b mt8173-elm-hana-6.6.15.dtb outf
when the inputs were unwriteable.

Link: https://bugs.debian.org/1063097
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 tools/fit_image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index beef1fa8..0fccfbb4 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -137,7 +137,7 @@ static int fdt_property_file(struct image_tool_params *params,
 	int ret;
 	int fd;
 
-	fd = open(fname, O_RDWR | O_BINARY);
+	fd = open(fname, O_RDONLY | O_BINARY);
 	if (fd < 0) {
 		fprintf(stderr, "%s: Can't open %s: %s\n",
 			params->cmdname, fname, strerror(errno));
-- 
2.39.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] tools: open FIT image for reading as O_RDONLY instead of O_RDWR

[Dragan Simic]

Hello Ahelenia,

Please see my comments below.

On 2024-03-20 14:08, Ahelenia Ziemiańska wrote:
> The only usage of the fd is for the single read() below;
> this prevented
> 	mkimage -f auto -A arm64 \
> 		-T kernel -C lz4 -d Image-6.6.15.lz4 \
> 		-b mt8173-elm-hana-6.6.15.dtb outf
> when the inputs were unwriteable.

s/unwriteable/unwritable/

Also, the patch subject could be moved to the patch description,
and the following could be used instead, as the subject:

     tools: open FIT image read-only

> 
> Link: https://bugs.debian.org/1063097
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Looking good to me.  Opening the file read-only is also good
from the standpoint of using least privilege to perform the
required operation.

With the small nitpicks above,

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  tools/fit_image.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index beef1fa8..0fccfbb4 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -137,7 +137,7 @@ static int fdt_property_file(struct
> image_tool_params *params,
>  	int ret;
>  	int fd;
> 
> -	fd = open(fname, O_RDWR | O_BINARY);
> +	fd = open(fname, O_RDONLY | O_BINARY);
>  	if (fd < 0) {
>  		fprintf(stderr, "%s: Can't open %s: %s\n",
>  			params->cmdname, fname, strerror(errno));

[PATCH v2] tools: open FIT image read-only

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

Open for reading as O_RDONLY instead of O_RDWR:
the only usage of the fd is for the single read() below;
this prevented
	mkimage -f auto -A arm64 \
		-T kernel -C lz4 -d Image-6.6.15.lz4 \
		-b mt8173-elm-hana-6.6.15.dtb outf
when the inputs were unwritable.

Link: https://bugs.debian.org/1063097
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
---
 tools/fit_image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index beef1fa8..0fccfbb4 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -137,7 +137,7 @@ static int fdt_property_file(struct image_tool_params *params,
 	int ret;
 	int fd;
 
-	fd = open(fname, O_RDWR | O_BINARY);
+	fd = open(fname, O_RDONLY | O_BINARY);
 	if (fd < 0) {
 		fprintf(stderr, "%s: Can't open %s: %s\n",
 			params->cmdname, fname, strerror(errno));
-- 
2.39.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] tools: open FIT image read-only

[Dragan Simic]

Hello all,

Just a brief reminder about merging this patch.  It looks fine to me.


On 2024-03-21 19:31, Ahelenia Ziemiańska wrote:
> Open for reading as O_RDONLY instead of O_RDWR:
> the only usage of the fd is for the single read() below;
> this prevented
> 	mkimage -f auto -A arm64 \
> 		-T kernel -C lz4 -d Image-6.6.15.lz4 \
> 		-b mt8173-elm-hana-6.6.15.dtb outf
> when the inputs were unwritable.
> 
> Link: https://bugs.debian.org/1063097
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  tools/fit_image.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index beef1fa8..0fccfbb4 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -137,7 +137,7 @@ static int fdt_property_file(struct
> image_tool_params *params,
>  	int ret;
>  	int fd;
> 
> -	fd = open(fname, O_RDWR | O_BINARY);
> +	fd = open(fname, O_RDONLY | O_BINARY);
>  	if (fd < 0) {
>  		fprintf(stderr, "%s: Can't open %s: %s\n",
>  			params->cmdname, fname, strerror(errno));

Re: [PATCH v2] tools: open FIT image read-only

[Tom Rini]

On Thu, 21 Mar 2024 19:31:54 +0100, Ahelenia Ziemiańska wrote:

> Open for reading as O_RDONLY instead of O_RDWR:
> the only usage of the fd is for the single read() below;
> this prevented
> 	mkimage -f auto -A arm64 \
> 		-T kernel -C lz4 -d Image-6.6.15.lz4 \
> 		-b mt8173-elm-hana-6.6.15.dtb outf
> when the inputs were unwritable.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom