Skip to content

[systemd-sonic-generator] Fix handling service files with additional fields under [Install] section#17764

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
stepanblyschak:fix-systemd-generator
Feb 29, 2024
Merged

[systemd-sonic-generator] Fix handling service files with additional fields under [Install] section#17764
liat-grozovik merged 1 commit intosonic-net:masterfrom
stepanblyschak:fix-systemd-generator

Conversation

@stepanblyschak
Copy link
Collaborator

If encountered a line without RequiredBy or WantedBy the code passes uninitialized pointer to get_install_targets_from_line(). Where it can fail with segfault or silently pass randomly.

Why I did it

Uninitialized target_suffix is passed to get_install_targets_from_line() when other fields are present in [Install] section, like this:

root@sonic:/home/admin# systemctl cat ntpsec
...
[Install]
Alias=ntp.service
Alias=ntpd.service
WantedBy=multi-user.target
Work item tracking
  • Microsoft ADO (number only):

How I did it

Initialize target_suffix with NULL, put an assert in get_install_targets_from_line(). Edited test to cover this scenario.

How to verify it

UT and on the switch.

Without the change - assert fails:

[       OK ] SsgFunctionTest.missing_file (2 ms)
[ RUN      ] SsgFunctionTest.insert_instance_number
[       OK ] SsgFunctionTest.insert_instance_number (2 ms)
[ RUN      ] SsgFunctionTest.get_num_of_asic
[       OK ] SsgFunctionTest.get_num_of_asic (2 ms)
[ RUN      ] SsgFunctionTest.get_unit_files
[       OK ] SsgFunctionTest.get_unit_files (2 ms)
[----------] 4 tests from SsgFunctionTest (8 ms total)

[----------] 4 tests from SsgMainTest
[ RUN      ] SsgMainTest.ssg_main_argv
Installation directory required as argument
[       OK ] SsgMainTest.ssg_main_argv (2 ms)
[ RUN      ] SsgMainTest.ssg_main_single_npu
ssg_test: systemd-sonic-generator.c:145: get_install_targets_from_line: Assertion `install_type' failed.
Aborted (core dumped)

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

…fields under [Install] section

Signed-off-by: Stepan Blyschak <[email protected]>
@liat-grozovik
Copy link
Collaborator

@stepanblyschak only only to 202305? what about 202311?
@theasianpianist kindly reminder to review and provide your feedback

@liat-grozovik liat-grozovik merged commit f30936d into sonic-net:master Feb 29, 2024
@keboliu
Copy link
Collaborator

keboliu commented Mar 12, 2024

@yxieca would you please help to cherry-pick?

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Mar 12, 2024
…fields under [Install] section (sonic-net#17764)

If encountered a line without RequiredBy or WantedBy the code passes uninitialized pointer to get_install_targets_from_line(). Where it can fail with segfault or silently pass randomly.

- Why I did it
Uninitialized target_suffix is passed to get_install_targets_from_line() when other fields are present in [Install] section, like this:

root@sonic:/home/admin# systemctl cat ntpsec
...
[Install]
Alias=ntp.service
Alias=ntpd.service
WantedBy=multi-user.target

- How I did it
Initialize target_suffix with NULL, put an assert in get_install_targets_from_line(). Edited test to cover this scenario.

- How to verify it
UT and on the switch.

Signed-off-by: Stepan Blyschak <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #18341

mssonicbld pushed a commit that referenced this pull request Mar 15, 2024
…fields under [Install] section (#17764)

If encountered a line without RequiredBy or WantedBy the code passes uninitialized pointer to get_install_targets_from_line(). Where it can fail with segfault or silently pass randomly.

- Why I did it
Uninitialized target_suffix is passed to get_install_targets_from_line() when other fields are present in [Install] section, like this:

root@sonic:/home/admin# systemctl cat ntpsec
...
[Install]
Alias=ntp.service
Alias=ntpd.service
WantedBy=multi-user.target

- How I did it
Initialize target_suffix with NULL, put an assert in get_install_targets_from_line(). Edited test to cover this scenario.

- How to verify it
UT and on the switch.

Signed-off-by: Stepan Blyschak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants