GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-116622: Don't expose `FICLONE` ioctl on Android by mhsmith · Pull Request #122522 · python/cpython

Conversation

mhsmith commented Jul 31, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

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.

Issue: Add support for Android as a target platform #116622

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Don't expose FICLONE ioctl on Android

fecac4b

mhsmith added the needs backport to 3.13 only security fixes label

Jul 31, 2024

bedevere-app Bot mentioned this pull request

Jul 31, 2024

Add support for Android as a target platform #116622

Closed

bedevere-app Bot added the awaiting review label

Jul 31, 2024

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting changes and removed awaiting review labels

Jul 31, 2024

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

freakboy3742 approved these changes

Jul 31, 2024

View reviewed changes

freakboy3742 left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok - those both make sense.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting merge and removed awaiting changes labels

Jul 31, 2024


          Merge branch 'main' into android-ficlone

5f16bbb

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.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the awaiting merge label

Aug 1, 2024

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 <[email protected]>
Co-authored-by: Russell Keith-Magee <[email protected]>

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.13 only 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 <[email protected]>
Co-authored-by: Russell Keith-Magee <[email protected]>

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 <[email protected]>

zaniebot mentioned this pull request

Oct 24, 2026

Fall back from FICLONE on permission errors and ENOTTY zanieb/cpython#60

Draft

Reviewers

freakboy3742

freakboy3742 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

mhsmith commented Jul 31, 2024 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Jul 31, 2024

Uh oh!

mhsmith commented Jul 31, 2024

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Aug 1, 2024

Uh oh!

bedevere-app Bot commented Aug 1, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mhsmith commented Jul 31, 2024 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.