Skip to content

Conversation

@AZero13
Copy link
Contributor

@AZero13 AZero13 commented Dec 18, 2025

Otherwise, fcntl can be called with invalid fd_flags.

Signed-off-by:Greg Funni [email protected]

@gitgitgadget-git
Copy link

There are issues in commit 0a5a44f:
open: check fd_flags value before calling fcntl
Commit not signed off

Otherwise, fcntl can be called with invalid fd_flags.

Signed-off-by: Greg Funni <[email protected]>
@AZero13
Copy link
Contributor Author

AZero13 commented Dec 18, 2025

/submit

@gitgitgadget-git
Copy link

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-git-2131/AZero13/fixopen-v1

To fetch this version to local tag pr-git-2131/AZero13/fixopen-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-git-2131/AZero13/fixopen-v1

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

"AZero13 via GitGitGadget" <[email protected]> writes:

> From: AZero13 <[email protected]>
>
> Otherwise, fcntl can be called with invalid fd_flags.

This somehow feels mischaracterised.  The intent of the code is
already to check the current flags value using getfd and or-in the
cloexec bit to call setfd.  What this patch fixes is to handle
a failed getfd case correctly.

    Subject: git_open_cloexec(): handle failing F_GETFD

    Before using F_SETFD to add in fd_cloexec bit, the code uses
    F_GETFD to see the current set of flags.  As it does not pay
    attention to potential failures, F_SETFD may be called with a
    set of invalid fcntl bits.

    Continue without calling F_SETFD and behave the same way as the
    case where F_SETFD failed, when the initial F_GETFD failed.

or something like that, perhaps?

> Signed-off-by: Greg Funni <[email protected]>

The in-body "From: AZer..." line we see above should say "From: Greg..."
instead.  Set "git config set user.name 'Greg Funni'" in the repository
you use to contribute to this project, amend the commit so that it will
record "Greg..." instead of "AZer..." as its author, and force push
to GGG and tell GGG to send out the email, perhaps?

> ---
>     open: check fd_flags value before calling fcntl
>     
>     Otherwise, fcntl can be called with invalid fd_flags.
>     
>     Signed-off-by:Greg Funni [email protected]
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2131%2FAZero13%2Ffixopen-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2131/AZero13/fixopen-v1
> Pull-Request: https://github.com/git/git/pull/2131
>
>  compat/open.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/compat/open.c b/compat/open.c
> index 37ae2b1aeb..b313bcd364 100644
> --- a/compat/open.c
> +++ b/compat/open.c
> @@ -44,8 +44,8 @@ int git_open_cloexec(const char *name, int flags)
>  
>  		if (!o_cloexec && 0 <= fd && fd_cloexec) {
>  			/* Opened w/o O_CLOEXEC?  try with fcntl(2) to add it */
> -			int flags = fcntl(fd, F_GETFD);
> -			if (fcntl(fd, F_SETFD, flags | fd_cloexec))
> +			int fd_flags = fcntl(fd, F_GETFD);
> +			if (fd_flags < 0 || fcntl(fd, F_SETFD, fd_flags | fd_cloexec))
>  				fd_cloexec = 0;
>  		}
>  	}
>
> base-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant