Skip to contentSkip to conversation

Do setuid before git operations

GIT PULL New Jack (Szu-Chieh) Wang 2 participants 6 messages May 13, 2022

lore
Share & tools
lore
#0 lore raw

From: "Jack (Szu-Chieh) Wang" <[email protected]>

/etc/build information missing is because the new git thinks it's unsafe to do git operations if the directory has a different uid from the executing process'. However, there are still cases that we are building the image with a different uid, making git reporting false alarms. This patch identifies the cases by attempting to setuid to the owner of the git directory and if this works, it should be the expected git repo thus safe to do git operations.

The following changes since commit 7b48f329aedc7fcb277302ba6ff167f22e5b1f22:

scripts/autobuilder-worker-prereq-tests: add additional limit testing (2022-05-11 19:59:44 +0100)

are available in the Git repository at:

git://git.yoctoproject.org/poky-contrib jackwang/setuid-before-git-op
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=jackwang/setuid-before-git-op

Jack Wang (1):
Do setuid before git operations

 meta/classes/metadata_scm.bbclass | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Signature · 1 line
2.36.0.550.gb090851708-goog
#1 lore raw

PATCH 1/1 Do setuid before git operations

After 2.35.2, git operations aren't considered safe when running in directories owned by other users.
This patch attempts to setuid to the uid of the directory owner before doing the git operations.

---
 meta/classes/metadata_scm.bbclass | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
meta/classes/metadata_scm.bbclass +6 -2
index 47cb969b8d..bb2f494533 100644
@@ -22,18 +22,22 @@ def base_get_metadata_svn_revision(path, d):
 
 def base_get_metadata_git_branch(path, d):
     import bb.process
+    import os
+    stat_info = os.stat(path)
 
     try:
-        rev, _ = bb.process.run('git rev-parse --abbrev-ref HEAD', cwd=path)
+        rev, _ = bb.process.run('git rev-parse --abbrev-ref HEAD', cwd=path, preexec_fn=lambda : os.setuid(stat_info.st_uid))
     except bb.process.ExecutionError:
         rev = '<unknown>'
     return rev.strip()
 
 def base_get_metadata_git_revision(path, d):
     import bb.process
+    import os
+    stat_info = os.stat(path)
 
     try:
-        rev, _ = bb.process.run('git rev-parse HEAD', cwd=path)
+        rev, _ = bb.process.run('git rev-parse HEAD', cwd=path, preexec_fn=lambda : os.setuid(stat_info.st_uid))
     except bb.process.ExecutionError:
         rev = '<unknown>'
     return rev.strip()
Signature · 1 line
2.36.0.550.gb090851708-goog
#2 lore raw
In reply to Jack (Szu-Chieh) Wang
Quoted 9 lines · Jack (Szu-Chieh) Wang via lists.yoctoproject.org

From: "Jack (Szu-Chieh) Wang" <[email protected]>

/etc/build information missing is because the new git thinks it's unsafe to do git operations if the directory has a different uid from the executing process'. However, there are still cases that we are building the image with a different uid, making git reporting false alarms. This patch identifies the cases by attempting to setuid to the owner of the git directory and if this works, it should be the expected git repo thus safe to do git operations.

This should have been fixed in master or in the process of being fixed on the stable branches by the git wrapper we've recently merged?

https://git.yoctoproject.org/poky/commit/?id=4d7383aefb391a5a998454c70feb96127951ca0a

and several other related commits.

Which release are you seeing that with?

Cheers,

Richard

#3 lore raw

Hi Richard,

Thanks for your prompt reply.
We're using the dunfell branch and the git issues remain. I see the patch is only on master not on dunfell, is this correct?

Sincerely,
Jack

Quoted 26 lines · On Fri, May 13, 2022 at 6:14 PM <[email protected]> wrote:
Quoted 9 lines · Jack (Szu-Chieh) Wang via lists.yoctoproject.org

From: "Jack (Szu-Chieh) Wang" <[email protected]>

/etc/build information missing is because the new git thinks it's unsafe to do git operations if the directory has a different uid from the executing process'. However, there are still cases that we are building the image with a different uid, making git reporting false alarms. This patch identifies the cases by attempting to setuid to the owner of the git directory and if this works, it should be the expected git repo thus safe to do git operations.

This should have been fixed in master or in the process of being fixed on the stable branches by the git wrapper we've recently merged?

https://git.yoctoproject.org/poky/commit/?id=4d7383aefb391a5a998454c70feb96127951ca0a

and several other related commits.

Which release are you seeing that with?

Cheers,

Richard

#4 lore raw
Quoted 5 lines · Jack (Szu-Chieh) Wang

Hi Richard,

Thanks for your prompt reply.
We're using the dunfell branch and the git issues remain. I see the patch is only on master not on dunfell, is this correct?

We have a backport policy which means we fix on master, then fixes cascade to the stable maintained releases. There is a review request out here:

http://cgit.openembedded.org/openembedded-core-contrib/log/?h=stable/dunfell-next

which includes the patches and should hopefully merge in the next few days.

Cheers,

Richard

#5 lore raw

I see, thanks a lot for your help and your efforts.

Sincerely,
Jack

Quoted 21 lines · On Fri, May 13, 2022 at 6:22 PM <[email protected]> wrote:
Quoted 5 lines · Jack (Szu-Chieh) Wang

Hi Richard,

Thanks for your prompt reply.
We're using the dunfell branch and the git issues remain. I see the patch is only on master not on dunfell, is this correct?

We have a backport policy which means we fix on master, then fixes cascade to the stable maintained releases. There is a review request out here:

http://cgit.openembedded.org/openembedded-core-contrib/log/?h=stable/dunfell-next

which includes the patches and should hopefully merge in the next few days.

Cheers,

Richard