FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-116622: Don't expose `FICLONE` ioctl on Android by mhsmith · Pull Request #122522 · python/cpython · GitHub

/ cpython Public

gh-116622: Don't expose FICLONE ioctl on Android - #122522

Merged
freakboy3742 merged 2 commits into
python:mainfrom
mhsmith:android-ficlone
Aug 1, 2024
ConversationCommits (2)ChecksFiles changed
Merged

gh-116622: Don't expose FICLONE ioctl on Android#122522
freakboy3742 merged 2 commits into
python:mainfrom
mhsmith:android-ficlone

Conversation

mhsmith commented Jul 31, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Although this ioctl exists in the system headers, it's blocked by SELinux with a message like this:

type=1400 audit(0.0:18729): avc: denied { ioctl } for path=2F646174612F646174612F6F72672E707974686F6E2E746573746265642F63616368652F746573745F707974686F6E5F776F726B65725F36373532C3A62F40746573745F363735325F746D70C3A62F636F707941 dev="dm-39" ino=369303 ioctlcmd=0x9409 scontext=u:r:untrusted_app:s0:c225,c256,c512,c768 tcontext=u:object_r:app_data_file:s0:c225,c256,c512,c768 tclass=file permissive=0 app=org.python.testbed

On Python 3.14 this breaks the test for Path.copy, which was added in #119058:

======================================================================
ERROR: test_copytree_to_existing_directory_dirs_exist_ok (test.test_pathlib.test_pathlib.PosixPathTest.test_copytree_to_existing_directory_dirs_exist_ok)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/test/test_pathlib/test_pathlib_abc.py", line 1899, in test_copytree_to_existing_directory_dirs_exist_ok
    source.copytree(target, dirs_exist_ok=True)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 870, in copytree
    on_error(err)
    ~~~~~~~~^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 848, in on_error
    raise err
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 868, in copytree
    on_error(err)
    ~~~~~~~~^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 848, in on_error
    raise err
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 864, in copytree
    source.copy(target_dir.joinpath(source.name),
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                follow_symlinks=follow_symlinks,
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                preserve_metadata=preserve_metadata)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_abc.py", line 827, in copy
    copyfileobj(source_f, target_f)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_os.py", line 174, in copyfileobj
    raise err
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_os.py", line 164, in copyfileobj
    raise err
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_os.py", line 160, in copyfileobj
    clonefd(source_fd, target_fd)
    ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/data/user/0/org.python.testbed/files/python/lib/python3.14/pathlib/_os.py", line 58, in clonefd
    fcntl.ioctl(target_fd, fcntl.FICLONE, source_fd)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
PermissionError: [Errno 13] Permission denied: '/data/data/org.python.testbed/cache/test_python_worker_7506æ/@test_7506_tmpæ/dirC/fileC' -> '/data/data/org.python.testbed/cache/test_python_worker_7506æ/@test_7506_tmpæ/copyC/fileC'

On Python 3.13 I don't think Python ever uses this ioctl itself, but it's still worth backporting this PR for the benefit of user code.

mhsmith added the needs backport to 3.13 bugs and security fixes label Jul 31, 2024
bedevere-app Bot mentioned this pull request Jul 31, 2024
Closed
mhsmith requested a review from freakboy3742 July 31, 2024 14:21
freakboy3742 requested changes Jul 31, 2024
View reviewed changes

freakboy3742 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A question and a comment:

Q: Where is the FICLONE and FICLONERANGE constant coming from? Could this be managed as an autoconf check, rather than an explicit Android #ifdef?

Comment: This needs docs noting the feature exclusion on Android.

bedevere-app Bot commented Jul 31, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

mhsmith commented Jul 31, 2024

Copy link
Copy Markdown
Member Author

Where is the FICLONE and FICLONERANGE constant coming from? Could this be managed as an autoconf check, rather than an explicit Android #ifdef?

They're coming from the operating system headers. fcntlmodule.c has #ifdefs for each one, so they don't need autoconf checks.

This needs docs noting the feature exclusion on Android.

I think this is already covered by the statement in the fcntl docs that "The values used for cmd are operating system dependent". There is no list of the available values in the Python documentation.

freakboy3742 approved these changes Jul 31, 2024
View reviewed changes

freakboy3742 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok - those both make sense.

freakboy3742 enabled auto-merge (squash) August 1, 2024 00:02
freakboy3742 merged commit 06656e2 into python:main Aug 1, 2024

miss-islington-app Bot commented Aug 1, 2024

Copy link
Copy Markdown

Thanks @mhsmith for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 1, 2024
pythongh-116622: Don't expose FICLONE ioctl on Android (pythonGH-12…
b405072 
…2522)

Don't expose `FICLONE` ioctl on Android

(cherry picked from commit 06656e2)

Co-authored-by: Malcolm Smith <smith@chaquo.com>
Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

bedevere-app Bot commented Aug 1, 2024

Copy link
Copy Markdown

GH-122539 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Aug 1, 2024
freakboy3742 added a commit that referenced this pull request Aug 16, 2024
[3.13] gh-116622: Don't expose FICLONE ioctl on Android (GH-122522) (…
69cf92f 
…#122539)

gh-116622: Don't expose `FICLONE` ioctl on Android (GH-122522)

Don't expose `FICLONE` ioctl on Android

(cherry picked from commit 06656e2)

Co-authored-by: Malcolm Smith <smith@chaquo.com>
Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
pythongh-116622: Don't expose FICLONE ioctl on Android (python#122522)
2c8eaba 
Don't expose `FICLONE` ioctl on Android

Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

freakboy3742 freakboy3742 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL