diff --git a/cereal b/cereal index 9327e4fbec..e137c5731e 160000 --- a/cereal +++ b/cereal @@ -1 +1 @@ -Subproject commit 9327e4fbec1a1b4e0658a18db4a6f332c1f53688 +Subproject commit e137c5731eb0264497c1f3b151fa030af5bb940a diff --git a/common/file_helpers.py b/common/file_helpers.py index a344a71fef..2037a665b0 100644 --- a/common/file_helpers.py +++ b/common/file_helpers.py @@ -39,28 +39,6 @@ def get_tmpdir_on_same_filesystem(path): return "/tmp" -class AutoMoveTempdir(): - def __init__(self, target_path, temp_dir=None): - self._target_path = target_path - self._path = tempfile.mkdtemp(dir=temp_dir) - - @property - def name(self): - return self._path - - def close(self): - os.rename(self._path, self._target_path) - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc_value, traceback): - if exc_type is None: - self.close() - else: - shutil.rmtree(self._path) - - class NamedTemporaryDir(): def __init__(self, temp_dir=None): self._path = tempfile.mkdtemp(dir=temp_dir) @@ -100,9 +78,7 @@ class CallbackReader: def _get_fileobject_func(writer, temp_dir): def _get_fileobject(): - file_obj = writer.get_fileobject(dir=temp_dir) - os.chmod(file_obj.name, 0o644) - return file_obj + return writer.get_fileobject(dir=temp_dir) return _get_fileobject @@ -122,20 +98,3 @@ def atomic_write_in_dir(path, **kwargs): """ writer = AtomicWriter(path, **kwargs) return writer._open(_get_fileobject_func(writer, os.path.dirname(path))) - - -def atomic_write_in_dir_neos(path, contents, mode=None): - """ - Atomically writes contents to path using a temporary file in the same directory - as path. Useful on NEOS, where `os.link` (required by atomic_write_in_dir) is missing. - """ - - f = tempfile.NamedTemporaryFile(delete=False, prefix=".tmp", dir=os.path.dirname(path)) - f.write(contents) - f.flush() - if mode is not None: - os.fchmod(f.fileno(), mode) - os.fsync(f.fileno()) - f.close() - - os.rename(f.name, path) diff --git a/common/tests/test_file_helpers.py b/common/tests/test_file_helpers.py index 0d81dec9bd..344455ba6c 100644 --- a/common/tests/test_file_helpers.py +++ b/common/tests/test_file_helpers.py @@ -14,7 +14,6 @@ class TestFileHelpers(unittest.TestCase): with open(path) as f: self.assertEqual(f.read(), "test") - self.assertEqual(os.stat(path).st_mode & 0o777, 0o644) os.remove(path) def test_atomic_write_on_fs_tmp(self): diff --git a/common/tests/test_params.py b/common/tests/test_params.py index 36bb6d96b5..0b3d8dbda0 100644 --- a/common/tests/test_params.py +++ b/common/tests/test_params.py @@ -1,9 +1,7 @@ -import os import threading import time import tempfile import shutil -import stat import unittest from common.params import Params, ParamKeyType, UnknownKeyName, put_nonblocking @@ -69,13 +67,6 @@ class TestParams(unittest.TestCase): with self.assertRaises(UnknownKeyName): self.params.put_bool("swag", True) - def test_params_permissions(self): - permissions = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IWGRP | stat.S_IROTH | stat.S_IWOTH - - self.params.put("DongleId", "cb38263377b873ee") - st_mode = os.stat(f"{self.tmpdir}/d/DongleId").st_mode - assert (st_mode & permissions) == permissions - def test_delete_not_there(self): assert self.params.get("CarParams") is None self.params.delete("CarParams") diff --git a/selfdrive/common/params.cc b/selfdrive/common/params.cc index cada3b8ad0..79f5f82cf2 100644 --- a/selfdrive/common/params.cc +++ b/selfdrive/common/params.cc @@ -52,25 +52,21 @@ int fsync_dir(const char* path) { return result; } -// TODO: replace by std::filesystem::create_directories int mkdir_p(std::string path) { char * _path = (char *)path.c_str(); - mode_t prev_mask = umask(0); for (char *p = _path + 1; *p; p++) { if (*p == '/') { *p = '\0'; // Temporarily truncate - if (mkdir(_path, 0777) != 0) { + if (mkdir(_path, 0775) != 0) { if (errno != EEXIST) return -1; } *p = '/'; } } - if (mkdir(_path, 0777) != 0) { + if (mkdir(_path, 0775) != 0) { if (errno != EEXIST) return -1; } - chmod(_path, 0777); - umask(prev_mask); return 0; } @@ -94,10 +90,6 @@ bool create_params_path(const std::string ¶m_path, const std::string &key_pa return false; } - if (chmod(tmp_dir, 0777) != 0) { - return false; - } - std::string link_path = std::string(tmp_dir) + ".link"; if (symlink(tmp_dir, link_path.c_str()) != 0) { return false; @@ -109,8 +101,7 @@ bool create_params_path(const std::string ¶m_path, const std::string &key_pa } } - // Ensure permissions are correct in case we didn't create the symlink - return chmod(key_path.c_str(), 0777) == 0; + return true; } void ensure_params_path(const std::string ¶ms_path) { @@ -265,8 +256,6 @@ int Params::put(const char* key, const char* value, size_t value_size) { break; } - // change permissions to 0666 for apks - if ((result = fchmod(tmp_fd, 0666)) < 0) break; // fsync to force persist the changes. if ((result = fsync(tmp_fd)) < 0) break; diff --git a/selfdrive/common/util.h b/selfdrive/common/util.h index 845e71614e..b0cd9ebb02 100644 --- a/selfdrive/common/util.h +++ b/selfdrive/common/util.h @@ -61,7 +61,7 @@ std::string dir_name(std::string const& path); // **** file fhelpers ***** std::string read_file(const std::string& fn); std::map read_files_in_dir(const std::string& path); -int write_file(const char* path, const void* data, size_t size, int flags = O_WRONLY, mode_t mode = 0777); +int write_file(const char* path, const void* data, size_t size, int flags = O_WRONLY, mode_t mode = 0664); std::string readlink(const std::string& path); bool file_exists(const std::string& fn); diff --git a/selfdrive/loggerd/logger.cc b/selfdrive/loggerd/logger.cc index dccc226a3d..9600319382 100644 --- a/selfdrive/loggerd/logger.cc +++ b/selfdrive/loggerd/logger.cc @@ -36,7 +36,7 @@ int logger_mkpath(char* file_path) { char* p; for (p=strchr(file_path+1, '/'); p; p=strchr(p+1, '/')) { *p = '\0'; - if (mkdir(file_path, 0777)==-1) { + if (mkdir(file_path, 0775)==-1) { if (errno != EEXIST) { *p = '/'; return -1; @@ -102,7 +102,7 @@ kj::Array logger_build_init_data() { init.setGitRemote(params_map["GitRemote"]); init.setPassive(params.getBool("Passive")); init.setDongleId(params_map["DongleId"]); - + auto lparams = init.initParams().initEntries(params_map.size()); int i = 0; for (auto& [key, value] : params_map) { @@ -145,8 +145,6 @@ static void log_sentinel(LoggerState *s, cereal::Sentinel::SentinelType type, in // ***** logging functions ***** void logger_init(LoggerState *s, const char* log_name, bool has_qlog) { - umask(0); - pthread_mutex_init(&s->lock, NULL); s->part = -1; diff --git a/selfdrive/loggerd/omx_encoder.cc b/selfdrive/loggerd/omx_encoder.cc index e1fbba64a6..bc2f745dbb 100644 --- a/selfdrive/loggerd/omx_encoder.cc +++ b/selfdrive/loggerd/omx_encoder.cc @@ -514,7 +514,7 @@ void OmxEncoder::encoder_open(const char* path) { // create camera lock file snprintf(this->lock_path, sizeof(this->lock_path), "%s/%s.lock", path, this->filename); - int lock_fd = open(this->lock_path, O_RDWR | O_CREAT, 0777); + int lock_fd = open(this->lock_path, O_RDWR | O_CREAT, 0664); assert(lock_fd >= 0); close(lock_fd); @@ -583,8 +583,8 @@ OmxEncoder::~OmxEncoder() { OMX_CHECK(OMX_FreeHandle(this->handle)); OMX_BUFFERHEADERTYPE *out_buf; - while (this->free_in.try_pop(out_buf)); - while (this->done_out.try_pop(out_buf)); + while (this->free_in.try_pop(out_buf)); + while (this->done_out.try_pop(out_buf)); if (this->codec_config) { free(this->codec_config); diff --git a/selfdrive/loggerd/raw_logger.cc b/selfdrive/loggerd/raw_logger.cc index cd27997737..5db5acf8cd 100644 --- a/selfdrive/loggerd/raw_logger.cc +++ b/selfdrive/loggerd/raw_logger.cc @@ -70,7 +70,7 @@ void RawLogger::encoder_open(const char* path) { LOG("open %s\n", lock_path.c_str()); - int lock_fd = open(lock_path.c_str(), O_RDWR | O_CREAT, 0777); + int lock_fd = open(lock_path.c_str(), O_RDWR | O_CREAT, 0664); assert(lock_fd >= 0); close(lock_fd); diff --git a/selfdrive/manager/manager.py b/selfdrive/manager/manager.py index a967a8ed88..bcbd74f991 100755 --- a/selfdrive/manager/manager.py +++ b/selfdrive/manager/manager.py @@ -55,8 +55,6 @@ def manager_init(): if params.get("Passive") is None: raise Exception("Passive must be set to continue") - os.umask(0) # Make sure we can create files with 777 permissions - # Create folders needed for msgq try: os.mkdir("/dev/shm") diff --git a/tools/lib/cache.py b/tools/lib/cache.py index 4018e06d30..82b2298730 100644 --- a/tools/lib/cache.py +++ b/tools/lib/cache.py @@ -1,6 +1,6 @@ import os import urllib.parse -from tools.lib.file_helpers import mkdirs_exists_ok +from common.file_helpers import mkdirs_exists_ok DEFAULT_CACHE_DIR = os.path.expanduser("~/.commacache") diff --git a/tools/lib/file_helpers.py b/tools/lib/file_helpers.py deleted file mode 100644 index a1595d43d7..0000000000 --- a/tools/lib/file_helpers.py +++ /dev/null @@ -1,26 +0,0 @@ -import os -from atomicwrites import AtomicWriter - - -def atomic_write_in_dir(path, **kwargs): - """Creates an atomic writer using a temporary file in the same directory - as the destination file. - """ - writer = AtomicWriter(path, **kwargs) - return writer._open(_get_fileobject_func(writer, os.path.dirname(path))) - - -def _get_fileobject_func(writer, temp_dir): - def _get_fileobject(): - file_obj = writer.get_fileobject(dir=temp_dir) - os.chmod(file_obj.name, 0o644) - return file_obj - return _get_fileobject - - -def mkdirs_exists_ok(path): - try: - os.makedirs(path) - except OSError: - if not os.path.isdir(path): - raise diff --git a/tools/lib/framereader.py b/tools/lib/framereader.py index b37e7b9c1a..00e28c9dfe 100644 --- a/tools/lib/framereader.py +++ b/tools/lib/framereader.py @@ -15,7 +15,7 @@ from lru import LRU import _io from tools.lib.cache import cache_path_for_file_path from tools.lib.exceptions import DataUnreadableError -from tools.lib.file_helpers import atomic_write_in_dir +from common.file_helpers import atomic_write_in_dir try: from xx.chffr.lib.filereader import FileReader diff --git a/tools/lib/url_file.py b/tools/lib/url_file.py index 7f111113ba..2161770d03 100644 --- a/tools/lib/url_file.py +++ b/tools/lib/url_file.py @@ -9,7 +9,7 @@ import pycurl from hashlib import sha256 from io import BytesIO from tenacity import retry, wait_random_exponential, stop_after_attempt -from tools.lib.file_helpers import mkdirs_exists_ok, atomic_write_in_dir +from common.file_helpers import mkdirs_exists_ok, atomic_write_in_dir # Cache chunk size K = 1000 CHUNK_SIZE = 1000 * K