Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hitting "File exists" on 2.6.0 #4546

Open
almusil opened this issue Oct 1, 2024 · 9 comments
Open

Hitting "File exists" on 2.6.0 #4546

almusil opened this issue Oct 1, 2024 · 9 comments

Comments

@almusil
Copy link

almusil commented Oct 1, 2024

Brief description

Scapy doesn't check if the .config directory already exists.

    from scapy.all import *  # noqa: F401,F403
    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/scapy/all.py", line 13, in <module>
    from scapy.data import *
  File "/usr/local/lib/python3.12/site-packages/scapy/data.py", line 415, in <module>
    @scapy_data_cache("ethertypes")
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/scapy/data.py", line 301, in scapy_data_cache
    from scapy.main import SCAPY_CACHE_FOLDER
  File "/usr/local/lib/python3.12/site-packages/scapy/main.py", line 165, in <module>
    SCAPY_CONFIG_FOLDER = _probe_config_folder("scapy")
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/scapy/main.py", line 82, in _probe_config_folder
    return _probe_xdg_folder(
           ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/scapy/main.py", line 76, in _probe_xdg_folder
    path.mkdir(mode=0o700)
  File "/usr/lib64/python3.12/pathlib.py", line 1311, in mkdir
    os.mkdir(self, mode)
FileExistsError: [Errno 17] File exists: '/root/.config'

Scapy version

2.6.0

Python version

3.12

Operating system

Linux 6.10.6

Additional environment information

No response

How to reproduce

Just importing from scapy.all import * is enough.

Actual result

No response

Expected result

No response

Related resources

No response

almusil added a commit to almusil/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
ovsrobot pushed a commit to ovsrobot/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
putnopvut pushed a commit to ovn-org/ovn that referenced this issue Oct 1, 2024
The latest scapy version (2.6.0) fails to properly evaulate if
.config directory already exists [0]. Pin the version to 2.5.0
for the time being as that one was working without any issues.

[0] secdev/scapy#4546
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
@KepptnKool
Copy link

I see a similar Error with the cache directory:

FileExistsError: [Errno 17] File exists: '/root/.cache'

@gpotter2
Copy link
Member

gpotter2 commented Oct 1, 2024

Scapy doesn't check if the .config directory already exists.

Sorry could you elaborate on why you are saying this? The code you mentioned in your stack trace seemed pretty clear to me, but I could be missing something:

scapy/scapy/main.py

Lines 71 to 76 in 79974f2

if not path.exists():
# ~ folder doesn't exist. Create according to spec
# https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
# "If, when attempting to write a file, the destination directory is
# non-existent an attempt should be made to create it with permission 0700."
path.mkdir(mode=0o700)

Maybe I'm missing something. What are the permissions of this folder on your machine?

@guedou
Copy link
Member

guedou commented Oct 1, 2024

I could not reproduce the issue either.

@almusil
Copy link
Author

almusil commented Oct 2, 2024

I didn't check the code, but the issue is clearly happening. It might be a race then, which for some reason didn't happen with previous version.

@gpotter2
Copy link
Member

gpotter2 commented Oct 2, 2024

What are the permissions of this folder on your machine?

Another thing to check: it's a folder, right?

@almusil
Copy link
Author

almusil commented Oct 2, 2024

When it happened it was a regular directory, I didn't check the permission, but it's only created by scapy so I would expect that it is 700.

@guedou
Copy link
Member

guedou commented Oct 2, 2024

Could you share more information to help us reproduce the issue ? For example, a docker image will be great, or your could isolate the problematic function an execute is outside of Scapy with different folders.

@KepptnKool
Copy link

KepptnKool commented Oct 2, 2024

I also see the issue in our CI. We run multiple processes that analyze pcap files in parallel. This might be race condition because if not path.exists(): and path.mkdir(mode=0o700) are not atomic. Multiple processes can pass the if statement but the actual file system operation has to run sequentially.
I suggest to run mkdir with exist_ok = True. This should also work when multiple scapy processes are run in parallel.

@almusil
Copy link
Author

almusil commented Oct 2, 2024

It is definitely a race condition, I was able to reproduce it after several tries with the following script in ghcr.io/ovn-org/ovn-tests@sha256:ffe496ad8ecb78cabf7986461645ca29af7078b255e298691bbab2ad34988469 image.

#!/bin/bash -e

cat << EOF > repro.py
from scapy.all import *
print("Test")
EOF

rm -rf /root/.config

for i in {0..150}; do
    python3 repro.py &
done

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

No branches or pull requests

4 participants