Skip to content

Commit bb777a0

Browse files
author
Kevin Lampis
committed
CP-53030: bootloader: stop MenuEntry.contents getting clobbered by setNextBoot
This makes MenuEntry.contents more useful as a way to inject arbitrary data into a menuentry. Before this change setNextBoot assumed it was the only user of this field as was clobbering any existing data. New method setGrubVariable() which will be used by an external tool. Signed-off-by: Kevin Lampis <[email protected]>
1 parent 5540b77 commit bb777a0

File tree

2 files changed

+123
-3
lines changed

2 files changed

+123
-3
lines changed

tests/test_bootloader.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,31 @@ def test_no_hypervisor(self):
5252
with self.assertRaises(RuntimeError):
5353
Bootloader.readGrub2("tests/data/grub-no-hypervisor.cfg")
5454

55+
def test_set_grub_variable(self):
56+
tmpdir = mkdtemp(prefix="testbl")
57+
env = os.path.join(tmpdir, 'grubenv')
58+
bl = Bootloader("", "", env_block=env)
59+
60+
self.assertFalse(os.path.isfile(env))
61+
62+
self.assertTrue(bl.setGrubVariable("waffles=true"))
63+
64+
self.assertTrue(os.path.isfile(env))
65+
self.assertGreater(os.path.getsize(env), 0)
66+
67+
def test_set_grub_variable_throws_without_envfile(self):
68+
bl = Bootloader("", "", env_block=None)
69+
70+
with self.assertRaises(AssertionError):
71+
bl.setGrubVariable("waffles=true")
72+
5573

5674
class TestMenuEntry(unittest.TestCase):
5775
def setUp(self):
5876
self.tmpdir = mkdtemp(prefix="testbl")
5977
self.fn = os.path.join(self.tmpdir, 'grub.cfg')
6078
self.bl = Bootloader('grub2', self.fn)
79+
self.env = os.path.join(self.tmpdir, 'grubenv')
6180

6281
def tearDown(self):
6382
shutil.rmtree(self.tmpdir)
@@ -137,6 +156,101 @@ def test_new_linux(self):
137156
}
138157
''')
139158

159+
def test_arbitrary_contents(self):
160+
""" Test that arbitrary data can be injected into the MenuEntry.contents field. """
161+
e = MenuEntry(hypervisor='xen.efi', hypervisor_args='a',
162+
kernel='vmlinuz', kernel_args='b',
163+
initrd='initrd.img',
164+
title='menu_name')
165+
166+
e.contents.append("\textra data line 1")
167+
e.contents.append("\textra data line 2")
168+
169+
e.entry_format = Grub2Format.XEN_BOOT
170+
171+
self.bl.append('menu_name', e)
172+
self.bl.commit()
173+
174+
with open_with_codec_handling(self.fn, 'r') as f:
175+
content = f.read()
176+
177+
self.assertEqual(content,
178+
'''menuentry 'menu_name' {
179+
extra data line 1
180+
extra data line 2
181+
xen_hypervisor xen.efi a
182+
xen_module vmlinuz b
183+
xen_module initrd.img
184+
}
185+
''')
186+
187+
def test_contents_not_clobbered_by_setnextboot(self):
188+
189+
self.assertIsNone(self.bl.env_block)
190+
self.bl.env_block = self.env
191+
192+
e = MenuEntry(hypervisor='xen.efi', hypervisor_args='a',
193+
kernel='vmlinuz', kernel_args='b',
194+
initrd='initrd.img',
195+
title='menu_title')
196+
197+
e.contents.append("\textra data line 1")
198+
e.contents.append("\textra data line 2")
199+
200+
e.entry_format = Grub2Format.XEN_BOOT
201+
202+
self.bl.append('menu_title', e)
203+
self.assertTrue(self.bl.setNextBoot('menu_title'))
204+
self.bl.commit()
205+
206+
207+
with open_with_codec_handling(self.fn, 'r') as f:
208+
content = f.read()
209+
210+
self.assertEqual(content,
211+
'''menuentry 'menu_title' {
212+
unset override_entry
213+
save_env override_entry
214+
extra data line 1
215+
extra data line 2
216+
xen_hypervisor xen.efi a
217+
xen_module vmlinuz b
218+
xen_module initrd.img
219+
}
220+
''')
221+
222+
def test_setnextboot_is_indempotent(self):
223+
self.bl.env_block = self.env
224+
225+
e = MenuEntry(hypervisor='xen.efi', hypervisor_args='a',
226+
kernel='vmlinuz', kernel_args='b',
227+
initrd='initrd.img',
228+
title='menu_title')
229+
230+
e.entry_format = Grub2Format.XEN_BOOT
231+
232+
self.bl.append('menu_title', e)
233+
234+
# Calling twice should have thte same effect as calling once
235+
self.assertTrue(self.bl.setNextBoot('menu_title'))
236+
self.assertTrue(self.bl.setNextBoot('menu_title'))
237+
238+
self.bl.commit()
239+
240+
241+
with open_with_codec_handling(self.fn, 'r') as f:
242+
content = f.read()
243+
244+
self.assertEqual(content,
245+
'''menuentry 'menu_title' {
246+
unset override_entry
247+
save_env override_entry
248+
xen_hypervisor xen.efi a
249+
xen_module vmlinuz b
250+
xen_module initrd.img
251+
}
252+
''')
253+
140254

141255
class TestLinuxBootloader(unittest.TestCase):
142256
def setUp(self):

xcp/bootloader.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,15 +374,21 @@ def setNextBoot(self, entry):
374374
return False
375375

376376
clear_default = ['\tunset override_entry', '\tsave_env override_entry']
377-
self.menu[entry].contents = clear_default
377+
if clear_default[0] not in self.menu[entry].contents:
378+
self.menu[entry].contents = clear_default + self.menu[entry].contents
378379

379380
for i in range(len(self.menu_order)):
380381
if self.menu_order[i] == entry:
381-
cmd = ['grub-editenv', self.env_block, 'set', 'override_entry=%d' % i]
382-
return xcp.cmd.runCmd(cmd) == 0
382+
return self.setGrubVariable('override_entry=%d' % i)
383383

384384
return False
385385

386+
def setGrubVariable(self, var):
387+
if self.env_block is None:
388+
raise AssertionError("No grubenv file")
389+
cmd = ['grub-editenv', self.env_block, 'set', var]
390+
return xcp.cmd.runCmd(cmd) == 0
391+
386392
@classmethod
387393
def newDefault(cls, kernel_link_name, initrd_link_name, root = '/'):
388394
b = cls.loadExisting(root)

0 commit comments

Comments
 (0)