From 75123c7b52ed1d45a09ebf6554e40090487d2ccd Mon Sep 17 00:00:00 2001 From: PJHsieh <49477291+PJHsieh@users.noreply.github.com> Date: Fri, 21 Feb 2025 02:11:08 +0000 Subject: [PATCH 1/2] Set the eeprom cache permissions to 755 Issue and Root Cause: The issue arises because, after a system reboot, the permissions of /var/cache/sonic/decode-syseeprom/ or its file /var/cache/sonic/decode-syseeprom/syseeprom_cache may be incorrectly set. This misconfiguration can cause the Platform API to require root privileges for access. This problem has been observed in various platforms, such as the AS7326_56X, where running show platform syseeprom results in errors due to permission issues. Solution: To address this, it's essential to ensure that the cache directory and its files have the correct permissions set upon system startup. Implementing a fix that adjusts these permissions appropriately can prevent the Platform API from requiring root privileges, thereby resolving the issue. --- sonic_platform_base/sonic_eeprom/eeprom_base.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sonic_platform_base/sonic_eeprom/eeprom_base.py b/sonic_platform_base/sonic_eeprom/eeprom_base.py index 8f1c6303f..de7f42418 100644 --- a/sonic_platform_base/sonic_eeprom/eeprom_base.py +++ b/sonic_platform_base/sonic_eeprom/eeprom_base.py @@ -295,9 +295,21 @@ def write_cache(self, e): if self.cache_name: F = None try: + # Ensure the directory exists + directory = os.path.dirname(self.cache_name) + + if not os.path.exists(directory): + os.makedirs(directory) + + # Set directory permissions to 755 + os.chmod(directory, 0o755) + F = open(self.cache_name, "wb") F.seek(self.s) F.write(e) + + # Set file permissions to 755 + os.chmod(self.cache_name, 0o755) except IOError as e: raise IOError("Failed to write cache : %s" % (str(e))) finally: From fb93dae8e9e016a3cb59bfce5b91bc95dc0fc6fa Mon Sep 17 00:00:00 2001 From: PJHsieh <49477291+PJHsieh@users.noreply.github.com> Date: Fri, 21 Feb 2025 06:11:31 +0000 Subject: [PATCH 2/2] Add unit test --- tests/eeprom_base_test.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/eeprom_base_test.py b/tests/eeprom_base_test.py index da89e2b95..1f7257b9e 100644 --- a/tests/eeprom_base_test.py +++ b/tests/eeprom_base_test.py @@ -2,7 +2,7 @@ import pytest import subprocess from unittest import mock -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, mock_open from sonic_platform_base.sonic_eeprom import eeprom_base, eeprom_tlvinfo EEPROM_SYMLINK = "vpd_info" EEPROM_HEX_FILE = "syseeprom.hex" @@ -200,6 +200,40 @@ def test_set_eeprom_invalid_field(self, mock_print): mock_print.assert_called_with("Error: invalid field '0x20'") assert e.value.code == 1 + @mock.patch("builtins.open", new_callable=mock.mock_open) + @mock.patch("os.makedirs") + @mock.patch("os.path.exists", return_value=False) + @mock.patch("os.chmod") + def test_write_cache(self, mock_chmod, mock_exists, mock_makedirs, mock_open): + # Create an instance of TlvInfoDecoder with a mock cache_name + eeprom_decoder = eeprom_tlvinfo.TlvInfoDecoder("/dev/null", 0, "", True) + eeprom_decoder.cache_name = "/tmp/mock_cache" + + # Mock EEPROM data to write + mock_eeprom_data = b"mock_eeprom_data" + + # Call the write_cache method + eeprom_decoder.write_cache(mock_eeprom_data) + + # Verify that os.makedirs was called with the correct directory + mock_makedirs.assert_called_once_with("/tmp") + + # Verify that os.chmod was called to set directory permissions + mock_chmod.assert_any_call("/tmp", 0o755) + + # Verify that the file was opened in write-binary mode + mock_open.assert_called_once_with("/tmp/mock_cache", "wb") + + # Verify that the file's seek method was called with the correct offset + mock_open().seek.assert_called_once_with(eeprom_decoder.s) + + # Verify that the file's write method was called with the EEPROM data + mock_open().write.assert_called_once_with(mock_eeprom_data) + + # Verify that os.chmod was called to set file permissions + mock_chmod.assert_any_call("/tmp/mock_cache", 0o755) + + def teardown(self): print("TEAR DOWN")