[PATCH] quiltimport: fix backslash expansion in patch subjects

[PATCH] quiltimport: fix backslash expansion in patch subjects

[Sasha Levin]

echo interprets backslash sequences, so a patch with "\0" in its
subject has that expanded into a NUL byte, which git commit-tree
rejects.

Use printf '%s\n' instead, which doesn't interpret the string.

Also quote $tmp_dir to handle paths with spaces.

Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 git-quiltimport.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index eb34cda409..38302d28c9 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
 # Find the initial commit
 commit=$(git rev-parse HEAD)
 
-mkdir $tmp_dir || exit 2
+mkdir "$tmp_dir" || exit 2
 while read patch_name level garbage <&3
 do
 	case "$patch_name" in ''|'#'*) continue;; esac
@@ -101,7 +101,7 @@ do
 		echo "$patch_name doesn't exist. Skipping."
 		continue
 	fi
-	echo $patch_name
+	printf '%s\n' "$patch_name"
 	git mailinfo $MAILINFO_OPT "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
 	test -s "$tmp_patch" || {
@@ -142,14 +142,14 @@ do
 	SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
 	export GIT_AUTHOR_DATE SUBJECT
 	if [ -z "$SUBJECT" ] ; then
-		SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
+		SUBJECT=$(printf '%s' "$patch_name" | sed -e 's/.patch$//')
 	fi
 
 	if [ -z "$dry_run" ] ; then
 		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
-		commit=$( { echo "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
+		commit=$( { printf '%s\n' "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
 	fi
 done 3<"$QUILT_SERIES"
-rm -rf $tmp_dir || exit 5
+rm -rf "$tmp_dir" || exit 5

base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
-- 
2.51.0

Re: [PATCH] quiltimport: fix backslash expansion in patch subjects

[Ben Knoble]

> Le 8 mars 2026 à 13:01, Sasha Levin <sashal@kernel.org> a écrit :
> 
> echo interprets backslash sequences, so a patch with "\0" in its
> subject has that expanded into a NUL byte, which git commit-tree
> rejects.

Echo _shouldn’t_ do that without flags like -e depending on your implementation, but I wouldn’t put it past echo ;) hence my preference for printf in scripts.

More interestingly, though: do the patch-names or subjects being adjusted below contain such a sequence? They look like user input, so possibly (and we can’t be sure they don’t).

It might be nice to say more about what “rejects” means (errors confusingly? Truncates user input?), but otherwise I expect this is reasonable.

> Use printf '%s\n' instead, which doesn't interpret the string.
> 
> Also quote $tmp_dir to handle paths with spaces.
> 
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> git-quiltimport.sh | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index eb34cda409..38302d28c9 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
> # Find the initial commit
> commit=$(git rev-parse HEAD)
> 
> -mkdir $tmp_dir || exit 2
> +mkdir "$tmp_dir" || exit 2

We prefer to leave such “while at it” changes in separate patches, so perhaps a preliminary cleanup “quote variable expansions to handle whitespace” or some such step would help?

(I didn’t look past the context to see if tmp_dir may have whitespace or shell meta characters.)

> while read patch_name level garbage <&3
> do
>    case "$patch_name" in ''|'#'*) continue;; esac
> @@ -101,7 +101,7 @@ do
>        echo "$patch_name doesn't exist. Skipping."
>        continue
>    fi
> -    echo $patch_name
> +    printf '%s\n' "$patch_name"

Does this go to commit-tree? Or just protecting the output for the user’s terminal?

>    git mailinfo $MAILINFO_OPT "$tmp_msg" "$tmp_patch" \
>        <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
>    test -s "$tmp_patch" || {
> @@ -142,14 +142,14 @@ do
>    SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
>    export GIT_AUTHOR_DATE SUBJECT
>    if [ -z "$SUBJECT" ] ; then
> -        SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
> +        SUBJECT=$(printf '%s' "$patch_name" | sed -e 's/.patch$//')

Interesting. I think POSIX sh  supports the ${x#suffix} expansion, which could avoid sed. I think it unlikely the “.” in the RE is intended to match any character rather than a literal dot. But should be done a separate patch and could be left for another series if you wanted (assuming my memory of supported expansions is correct).

Importantly, this does get fed to commit-tree below…

>    fi
> 
>    if [ -z "$dry_run" ] ; then
>        git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
>        tree=$(git write-tree) &&
> -        commit=$( { echo "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
> +        commit=$( { printf '%s\n' "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
>        git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
>    fi

… so may need protected as described. Neat. 

> done 3<"$QUILT_SERIES"
> -rm -rf $tmp_dir || exit 5
> +rm -rf "$tmp_dir" || exit 5

Ditto for cleanup. 

> base-commit: 795c338de725e13bd361214c6b768019fc45a2c1
> --
> 2.51.0

Thanks

Re: [PATCH] quiltimport: fix backslash expansion in patch subjects

[Sasha Levin]

Thanks for the review Ben! I'll send a v2 based on your review.

More explanation below:

On Sun, Mar 08, 2026 at 04:56:57PM -0400, Ben Knoble wrote:
>> Le 8 mars 2026 à 13:01, Sasha Levin <sashal@kernel.org> a écrit :
>>
>> echo interprets backslash sequences, so a patch with "\0" in its
>> subject has that expanded into a NUL byte, which git commit-tree
>> rejects.
>
>Echo _shouldn’t_ do that without flags like -e depending on your implementation, but I wouldn’t put it past echo ;) hence my preference for printf in scripts.

Right, this is just dash being "funny" and interprets backslash sequences in
echo by default.

sasha@laps:~$ dash -c 'echo "hello \0 world"' | xxd
00000000: 6865 6c6c 6f20 0020 776f 726c 640a       hello . world.
sasha@laps:~$ bash -c 'echo "hello \0 world"' | xxd
00000000: 6865 6c6c 6f20 5c30 2077 6f72 6c64 0a    hello \0 world.

>More interestingly, though: do the patch-names or subjects being adjusted below contain such a sequence? They look like user input, so possibly (and we can’t be sure they don’t).

Yes, this was hit in practice. A patch in the Linux kernel stable queue was
titled "selftests: tc_actions: don't dump 2MB of \0 to stdout". The literal \0
in the subject caused git-quiltimport to abort mid-import under dash.

>It might be nice to say more about what “rejects” means (errors confusingly? Truncates user input?), but otherwise I expect this is reasonable.

commit_tree_extended() in commit.c does:

     if (memchr(msg, '\0', msg_len))
         return error("a NUL byte in commit log message not allowed.");

So commit-tree errors out, which triggers the "|| exit 4" in the script and
aborts the entire quiltimport. Everything after the offending patch in the
series is silently lost. I'll add it to the commit message.

>> Use printf '%s\n' instead, which doesn't interpret the string.
>>
>> Also quote $tmp_dir to handle paths with spaces.
>>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>> git-quiltimport.sh | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
>> index eb34cda409..38302d28c9 100755
>> --- a/git-quiltimport.sh
>> +++ b/git-quiltimport.sh
>> @@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
>> # Find the initial commit
>> commit=$(git rev-parse HEAD)
>>
>> -mkdir $tmp_dir || exit 2
>> +mkdir "$tmp_dir" || exit 2
>
>We prefer to leave such “while at it” changes in separate patches, so perhaps a preliminary cleanup “quote variable expansions to handle whitespace” or some such step would help?
>
>(I didn’t look past the context to see if tmp_dir may have whitespace or shell meta characters.)

Will do: I'll split the $tmp_dir quoting into a preliminary cleanup patch.

>> while read patch_name level garbage <&3
>> do
>>    case "$patch_name" in ''|'#'*) continue;; esac
>> @@ -101,7 +101,7 @@ do
>>        echo "$patch_name doesn't exist. Skipping."
>>        continue
>>    fi
>> -    echo $patch_name
>> +    printf '%s\n' "$patch_name"
>
>Does this go to commit-tree? Or just protecting the output for the user’s terminal?

Just terminal output. It doesn't feed into commit-tree, but under dash a patch
name with backslash sequences would still produce garbled output, so worth
fixing alongside.

>>    git mailinfo $MAILINFO_OPT "$tmp_msg" "$tmp_patch" \
>>        <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
>>    test -s "$tmp_patch" || {
>> @@ -142,14 +142,14 @@ do
>>    SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
>>    export GIT_AUTHOR_DATE SUBJECT
>>    if [ -z "$SUBJECT" ] ; then
>> -        SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
>> +        SUBJECT=$(printf '%s' "$patch_name" | sed -e 's/.patch$//')
>
>Interesting. I think POSIX sh  supports the ${x#suffix} expansion, which could avoid sed. I think it unlikely the “.” in the RE is intended to match any character rather than a literal dot. But should be done a separate patch and could be left for another series if you wanted (assuming my memory of supported expansions is correct).
>
>Importantly, this does get fed to commit-tree below…

Good catch! ${patch_name%.patch} is POSIX and already used throughout git's
shell scripts.

>>    fi
>>
>>    if [ -z "$dry_run" ] ; then
>>        git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
>>        tree=$(git write-tree) &&
>> -        commit=$( { echo "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
>> +        commit=$( { printf '%s\n' "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
>>        git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
>>    fi
>
>… so may need protected as described. Neat.
>
>> done 3<"$QUILT_SERIES"
>> -rm -rf $tmp_dir || exit 5
>> +rm -rf "$tmp_dir" || exit 5
>
>Ditto for cleanup.

ack!

-- 
Thanks,
Sasha

Re: [PATCH] quiltimport: fix backslash expansion in patch subjects

[Junio C Hamano]

Sasha Levin <sashal@kernel.org> writes:

> echo interprets backslash sequences, so a patch with "\0" in its

"Some implementations of echo"; I think it is in XSI but the
plain vanilla POSIX makes it "implementation-defined".

> subject has that expanded into a NUL byte, which git commit-tree
> rejects.
>
> Use printf '%s\n' instead, which doesn't interpret the string.
>
> Also quote $tmp_dir to handle paths with spaces.
>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  git-quiltimport.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index eb34cda409..38302d28c9 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
>  # Find the initial commit
>  commit=$(git rev-parse HEAD)
>  
> -mkdir $tmp_dir || exit 2
> +mkdir "$tmp_dir" || exit 2
>  while read patch_name level garbage <&3
>  do
>  	case "$patch_name" in ''|'#'*) continue;; esac
> @@ -101,7 +101,7 @@ do
>  		echo "$patch_name doesn't exist. Skipping."
>  		continue
>  	fi
> -	echo $patch_name
> +	printf '%s\n' "$patch_name"

Unquoted "$patch_name" in the original fed to "echo" is doing more
than just backslash 0.

    patch_name="My   casual patch"

would be split into three tokens, runs of multiple spaces in the
original will be squashed into one, and "My casual patch" would have
been the result, which people may have appreciated as cleaning up a
sloppy original patch title.

The updated version will give completely different result, losing
the "cleaning up" feature and parrotting the garbage input to
garbage output.

So I am not 100% convinced that this change would not result in
robbing Peter to pay Paul.

Likewise for the next hunk.

Thanks.

> @@ -142,14 +142,14 @@ do
>  	SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
>  	export GIT_AUTHOR_DATE SUBJECT
>  	if [ -z "$SUBJECT" ] ; then
> -		SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
> +		SUBJECT=$(printf '%s' "$patch_name" | sed -e 's/.patch$//')
>  	fi
>  
>  	if [ -z "$dry_run" ] ; then
>  		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
>  		tree=$(git write-tree) &&
> -		commit=$( { echo "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
> +		commit=$( { printf '%s\n' "$SUBJECT"; echo; cat "$tmp_msg"; } | git commit-tree $tree -p $commit) &&
>  		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
>  	fi
>  done 3<"$QUILT_SERIES"
> -rm -rf $tmp_dir || exit 5
> +rm -rf "$tmp_dir" || exit 5
>
> base-commit: 795c338de725e13bd361214c6b768019fc45a2c1

Re: [PATCH] quiltimport: fix backslash expansion in patch subjects

[Sasha Levin]

On Sun, Mar 08, 2026 at 05:38:52PM -0700, Junio C Hamano wrote:
>Sasha Levin <sashal@kernel.org> writes:
>
>> echo interprets backslash sequences, so a patch with "\0" in its
>
>"Some implementations of echo"; I think it is in XSI but the
>plain vanilla POSIX makes it "implementation-defined".

I had to Google this one :)

I'll reword to "Some implementations of echo (notably dash) interpret backslash
sequences by default; POSIX leaves this behavior implementation-defined."

>> subject has that expanded into a NUL byte, which git commit-tree
>> rejects.
>>
>> Use printf '%s\n' instead, which doesn't interpret the string.
>>
>> Also quote $tmp_dir to handle paths with spaces.
>>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>>  git-quiltimport.sh | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
>> index eb34cda409..38302d28c9 100755
>> --- a/git-quiltimport.sh
>> +++ b/git-quiltimport.sh
>> @@ -79,7 +79,7 @@ tmp_info="$tmp_dir/info"
>>  # Find the initial commit
>>  commit=$(git rev-parse HEAD)
>>
>> -mkdir $tmp_dir || exit 2
>> +mkdir "$tmp_dir" || exit 2
>>  while read patch_name level garbage <&3
>>  do
>>  	case "$patch_name" in ''|'#'*) continue;; esac
>> @@ -101,7 +101,7 @@ do
>>  		echo "$patch_name doesn't exist. Skipping."
>>  		continue
>>  	fi
>> -	echo $patch_name
>> +	printf '%s\n' "$patch_name"
>
>Unquoted "$patch_name" in the original fed to "echo" is doing more
>than just backslash 0.
>
>    patch_name="My   casual patch"
>
>would be split into three tokens, runs of multiple spaces in the
>original will be squashed into one, and "My casual patch" would have
>been the result, which people may have appreciated as cleaning up a
>sloppy original patch title.
>
>The updated version will give completely different result, losing
>the "cleaning up" feature and parrotting the garbage input to
>garbage output.
>
>So I am not 100% convinced that this change would not result in
>robbing Peter to pay Paul.

patch_name is read from the series file via:

     while read patch_name level garbage <&3

Since read splits on IFS, patch_name only ever gets the first
whitespace delimited token. So in the normal case it can never
contain internal whitespace, and the word splitting in echo is a
noop:

     $ printf 'my-patch.patch -p1\n' |
       while read name level garbage; do
         echo "name=[$name]"
       done
     name=[my-patch.patch]

The only way patch_name gets internal spaces is if the series file
uses "\ " escaping, but in that case the spaces should be (??)
intentional.

>Likewise for the next hunk.

Same reasoning applies to the SUBJECT fallback, I think...

     SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')

patch_name is still the same single token from read. And the old path was
already quoted:

     echo "$SUBJECT"

So the only issue there was backslash interpretation, rather than splitting?
similar to 47be066026 ("rebase -i: do not "echo" random user-supplied
strings")?

I need a beer.

-- 
Thanks,
Sasha