oxidecomputer / humility

Debugger for Hubris
Mozilla Public License 2.0
527 stars 50 forks source link

UdpDumpAgent does not check image ID, sowing confusion #395

Closed bcantrill closed 1 year ago

bcantrill commented 1 year ago

Currently, the UdpDumpAgent does not check the image ID -- which means that it will read memory when the archive does not in fact match the image,. e.g.:

$ humility --ip 'fe80::c1d:7dff:fe2c:9295%rack2sw0tp1' -a /staff/dock/rack2/mupdate-20230609/build-gimlet-c-image-default.zip tasks
humility: connecting to fe80::c1d:7dff:fe2c:9295%rack2sw0tp1
humility: reading tasks remotely; state may not be consistent
system time = unavailable-via-net
ID TASK                       GEN PRI STATE    
 0 jefe                         ?   0 [cannot read supervisor memory]
 1 net                          0   5 recv, notif: eth-irq(irq61) wake-timer(T=37518762)
 2 sys                          0   1 recv
 3 spi2_driver                  0   3 recv
 4 i2c_driver                   0   3 recv
 5 spd                          0   2 notif: i2c1-irq(irq31/irq32)
 6 packrat                      0   1 recv
 7 thermal                      0   5 recv, notif: timer(T=37519468)
 8 power                        0   6 recv, notif: timer(T=37518953)
 9 hiffy                        0   5 notif: bit31(T=37518895)
10 gimlet_seq                   0   4 recv, notif: timer(T=37518663)
11 hash_driver                  0   2 recv
12 hf                           0   3 recv
13 update_server                0   3 recv
14 sensor                       0   4 recv, notif: timer(T=37519002)
15 host_sp_comms                0   7 recv, notif: jefe-state-change usart-irq(irq82) multitimer control-plane-agent
16 udpecho                      0   6 notif: socket
17 udpbroadcast                 0   6 notif: bit31(T=37519143)
18 udprpc                       0   6 notif: socket
19 control_plane_agent          0   6 recv, notif: usart-irq(irq37) socket timer
20 sprot                        0   4 recv
21 validate                     0   5 recv
22 vpd                          0   5 recv
23 user_leds                    0   2 recv, notif: timer
24 dump_agent                   0   6 wait: reply from jefe/gen0
25 sbrmi                        0   4 recv
26 idle                         0   8 ready
$ humility --ip 'fe80::c1d:7dff:fe2c:9295%rack2sw0tp1' -a /staff/dock/rack2/mupdate-20230609/build-gimlet-c-image-default.zip hiffy -c Jefe.get_state
humility: connecting to fe80::c1d:7dff:fe2c:9295%rack2sw0tp1
humility hiffy failed: RPC error: BadImageId: [34, 62, f5, 03, 97, e4, c6, 20] (Humility) [67, 4a, ee, b0, 93, 60, 2f, c0] (Hubris)

This can be especially confusing because -- by default -- humility dump will use the UdpDumpAgent, which will result in a dump that is internally inconsistent as the dumped image doesn't necessarily correspond to the archive. (@rmustacc can into this particular pathology, and it is naturally very confusing!)

It seems that UdpDumpAgent should likely do what we do with the udprpc RpcHeader and have an image ID in the Header (currently, humpty::udp::Header has a version and message ID, but no image ID). Alternatively (and perhaps friendlier, from a backwards compatibility perspective), we could add a new humpty::udp::Request that asks for or sends the image ID. Either way, we shouldn't proceed if the archive doesn't match the image ID of the running system.

bcantrill commented 1 year ago

Fixed in #397.