diff mbox series

[pseudo,1/1] pseudo_util.c: Open file with O_CLOEXEC to avoid fd leak

Message ID 20230301000302.93660-2-yoann.congal@smile.fr
State New
Headers show
Series pseudo_util.c: Open file with O_CLOEXEC to avoid fd leak | expand

Commit Message

Yoann Congal March 1, 2023, 12:03 a.m. UTC 
From: Pavel Modilaynen <pavelmn@axis.com>

Use close-on-exec (O_CLOEXEC) flag when open log file to
make sure its file descriptor is not leaked to parent
process on fork/exec.

Fixes [YOCTO #13311]

Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
---
 pseudo_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexandre Belloni March 1, 2023, 8:52 a.m. UTC | #1 
On 01/03/2023 01:03:04+0100, Yoann Congal wrote:
> From: Pavel Modilaynen <pavelmn@axis.com>
> 
> Use close-on-exec (O_CLOEXEC) flag when open log file to
> make sure its file descriptor is not leaked to parent
> process on fork/exec.
> 
> Fixes [YOCTO #13311]
> 

This is missing the author's SoB

> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> ---
>  pseudo_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pseudo_util.c b/pseudo_util.c
> index 64636b7..b58036f 100644
> --- a/pseudo_util.c
> +++ b/pseudo_util.c
> @@ -1611,7 +1611,7 @@ pseudo_logfile(char *filename, char *defname, int prefer_fd) {
>  		}
>  		free(filename);
>  	}	
> -	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644);
> +	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
>  	if (fd == -1) {
>  		pseudo_diag("help: can't open log file %s: %s\n", pseudo_path, strerror(errno));
>  	} else {
> -- 
> 2.30.2
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#177861): https://lists.openembedded.org/g/openembedded-core/message/177861
> Mute This Topic: https://lists.openembedded.org/mt/97303652/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Yoann Congal March 1, 2023, 9 a.m. UTC | #2 
Hi,

On 3/1/23 09:52, Alexandre Belloni wrote:
> On 01/03/2023 01:03:04+0100, Yoann Congal wrote:
>> From: Pavel Modilaynen <pavelmn@axis.com>
>>
>> Use close-on-exec (O_CLOEXEC) flag when open log file to
>> make sure its file descriptor is not leaked to parent
>> process on fork/exec.
>>
>> Fixes [YOCTO #13311]
>>
> 
> This is missing the author's SoB

That is right. But, as you would have noticed by now, his mail address is not valid anymore...

What is the proper way to handle this?
* Should I track him to ask him a proper "Signed-off" patch ? (He seems to have changed company)
* Should I change the author of the patch? (I do not like this but on the other hand, the change is quite trivial)
* Something I have not thought of?

> 
>> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
>> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
>> ---
>>  pseudo_util.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/pseudo_util.c b/pseudo_util.c
>> index 64636b7..b58036f 100644
>> --- a/pseudo_util.c
>> +++ b/pseudo_util.c
>> @@ -1611,7 +1611,7 @@ pseudo_logfile(char *filename, char *defname, int prefer_fd) {
>>  		}
>>  		free(filename);
>>  	}	
>> -	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644);
>> +	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
>>  	if (fd == -1) {
>>  		pseudo_diag("help: can't open log file %s: %s\n", pseudo_path, strerror(errno));
>>  	} else {
>> -- 
>> 2.30.2
>>
> 
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#177861): https://lists.openembedded.org/g/openembedded-core/message/177861
>> Mute This Topic: https://lists.openembedded.org/mt/97303652/3617179
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
> 
>
Alexander Kanavin March 1, 2023, 9:04 a.m. UTC | #3 
I believe we need only the submitter's s-o-b, not the author?

Alex

On Wed, 1 Mar 2023 at 10:01, Yoann Congal <yoann.congal@smile.fr> wrote:
>
> Hi,
>
> On 3/1/23 09:52, Alexandre Belloni wrote:
> > On 01/03/2023 01:03:04+0100, Yoann Congal wrote:
> >> From: Pavel Modilaynen <pavelmn@axis.com>
> >>
> >> Use close-on-exec (O_CLOEXEC) flag when open log file to
> >> make sure its file descriptor is not leaked to parent
> >> process on fork/exec.
> >>
> >> Fixes [YOCTO #13311]
> >>
> >
> > This is missing the author's SoB
>
> That is right. But, as you would have noticed by now, his mail address is not valid anymore...
>
> What is the proper way to handle this?
> * Should I track him to ask him a proper "Signed-off" patch ? (He seems to have changed company)
> * Should I change the author of the patch? (I do not like this but on the other hand, the change is quite trivial)
> * Something I have not thought of?
>
> >
> >> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> >> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> >> ---
> >>  pseudo_util.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/pseudo_util.c b/pseudo_util.c
> >> index 64636b7..b58036f 100644
> >> --- a/pseudo_util.c
> >> +++ b/pseudo_util.c
> >> @@ -1611,7 +1611,7 @@ pseudo_logfile(char *filename, char *defname, int prefer_fd) {
> >>              }
> >>              free(filename);
> >>      }
> >> -    fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644);
> >> +    fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
> >>      if (fd == -1) {
> >>              pseudo_diag("help: can't open log file %s: %s\n", pseudo_path, strerror(errno));
> >>      } else {
> >> --
> >> 2.30.2
> >>
> >
> >>
> >>
> >>
> >
> >
>
> --
> Yoann Congal
> Smile ECS - Tech Expert
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#177877): https://lists.openembedded.org/g/openembedded-core/message/177877
> Mute This Topic: https://lists.openembedded.org/mt/97303652/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie March 1, 2023, 10:16 a.m. UTC | #4 
On Wed, 2023-03-01 at 10:00 +0100, Yoann Congal wrote:
> Hi,
> 
> On 3/1/23 09:52, Alexandre Belloni wrote:
> > On 01/03/2023 01:03:04+0100, Yoann Congal wrote:
> > > From: Pavel Modilaynen <pavelmn@axis.com>
> > > 
> > > Use close-on-exec (O_CLOEXEC) flag when open log file to
> > > make sure its file descriptor is not leaked to parent
> > > process on fork/exec.
> > > 
> > > Fixes [YOCTO #13311]
> > > 
> > 
> > This is missing the author's SoB
> 
> That is right. But, as you would have noticed by now, his mail address is not valid anymore...
> 
> What is the proper way to handle this?
> * Should I track him to ask him a proper "Signed-off" patch ? (He seems to have changed company)
> * Should I change the author of the patch? (I do not like this but on the other hand, the change is quite trivial)
> * Something I have not thought of?

We need the submitters SoB line, it doesn't have to have the author's
although obviously it is better if we can. Since we can't here it just
means others have to verify the terms of the contribution. The patch is
trivial so it should all be fine.

Cheers,

Richard
Richard Purdie March 1, 2023, 10:18 a.m. UTC | #5 
On Wed, 2023-03-01 at 01:03 +0100, Yoann Congal wrote:
> From: Pavel Modilaynen <pavelmn@axis.com>
> 
> Use close-on-exec (O_CLOEXEC) flag when open log file to
> make sure its file descriptor is not leaked to parent
> process on fork/exec.
> 
> Fixes [YOCTO #13311]
> 
> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> ---
>  pseudo_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pseudo_util.c b/pseudo_util.c
> index 64636b7..b58036f 100644
> --- a/pseudo_util.c
> +++ b/pseudo_util.c
> @@ -1611,7 +1611,7 @@ pseudo_logfile(char *filename, char *defname, int prefer_fd) {
>  		}
>  		free(filename);
>  	}	
> -	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644);
> +	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
>  	if (fd == -1) {
>  		pseudo_diag("help: can't open log file %s: %s\n", pseudo_path, strerror(errno));
>  	} else {

Thanks. I've put this on the oe-core-next branch in the pseudo repo.
I'm hoping Alexandre could run a test build please?

Cheers,

Richard
diff mbox series

Patch

diff --git a/pseudo_util.c b/pseudo_util.c
index 64636b7..b58036f 100644
--- a/pseudo_util.c
+++ b/pseudo_util.c
@@ -1611,7 +1611,7 @@  pseudo_logfile(char *filename, char *defname, int prefer_fd) {
 		}
 		free(filename);
 	}	
-	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT, 0644);
+	fd = open(pseudo_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0644);
 	if (fd == -1) {
 		pseudo_diag("help: can't open log file %s: %s\n", pseudo_path, strerror(errno));
 	} else {