Skip to content

Commit ddfe39b

Browse files
committed
[rescue] Report errors for disallowed configurations
Rework the `validate_mode` function to set flags in the DFU context that represent whether a DFU `DnLoad` or `UpLoad` operation is allowed. Report an error in the download/upload handler when an unsupported operation is attempted. Signed-off-by: Chris Frantz <cfrantz@google.com>
1 parent 573efe9 commit ddfe39b

File tree

2 files changed

+50
-37
lines changed

2 files changed

+50
-37
lines changed

sw/device/silicon_creator/lib/rescue/dfu.c

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,29 @@
1515
#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"
1616

1717
typedef struct rescue_mode_properties {
18+
// The FourCC of the mode we want to set.
1819
uint32_t mode;
19-
bool dnload;
20-
bool upload;
20+
// Non-zero if therre is a paired mode.
21+
uint32_t paired;
22+
// Whether the mode support DFU DnLoad or UpLoad or both.
23+
// Element 0 describes the Up- or Dn-Load status of the mode.
24+
// Element 1 describes whether the "paired" mode supports Up- or Dn-Load.
25+
// E.g. OwnerPage0 is paired with OwnerBlock.
26+
uint8_t allow[2];
2127
} rescue_mode_properties_t;
2228

29+
// clang-format off
2330
static const rescue_mode_properties_t mode_by_altsetting[] = {
24-
{kRescueModeFirmware, true, false},
25-
{kRescueModeFirmwareSlotB, true, false},
26-
{kRescueModeOpenTitanID, false, true},
27-
{kRescueModeBootLog, false, true},
28-
{kRescueModeBootSvcRsp, true, true},
29-
{kRescueModeOwnerPage0, true, true},
30-
//{ kRescueModeOwnerPage1, false, true },
31+
{kRescueModeFirmware, 0, { kDfuAllowDnLoad, 0 }},
32+
{kRescueModeFirmwareSlotB, 0, { kDfuAllowDnLoad, 0 }},
33+
{kRescueModeOpenTitanID, 0, { kDfuAllowUpLoad, 0 }},
34+
{kRescueModeBootLog, 0, { kDfuAllowUpLoad, 0 }},
35+
{kRescueModeBootSvcRsp, kRescueModeBootSvcReq, { kDfuAllowUpLoad, kDfuAllowDnLoad }},
36+
{kRescueModeOwnerPage0, kRescueModeOwnerBlock, { kDfuAllowUpLoad, kDfuAllowDnLoad }},
3137
};
38+
// clang-format on
3239

33-
static rom_error_t validate_mode(uint32_t setting, rescue_state_t *state) {
40+
static rom_error_t validate_mode(uint32_t setting, dfu_ctx_t *ctx) {
3441
// Allow the `setting` to be either an index or a FourCC code.
3542
// The the integer value is less than the arraysize, then its clearly an
3643
// index.
@@ -56,38 +63,37 @@ static rom_error_t validate_mode(uint32_t setting, rescue_state_t *state) {
5663
// same target, we handle the "send" services first to stage data into the
5764
// rescue buffer.
5865
const rescue_mode_properties_t *mode = &mode_by_altsetting[setting];
59-
rom_error_t error2 = kErrorOk;
60-
rom_error_t error = rescue_validate_mode(mode->mode, state);
66+
rom_error_t error = rescue_validate_mode(mode->mode, &ctx->state);
6167

62-
// If the service exclusively supports either upload or download operation,
63-
// report bad mode immediately if a prior error occurred.
64-
if (!(mode->upload && mode->dnload)) {
65-
if (error != kErrorOk) {
66-
return kErrorRescueBadMode;
68+
uint8_t allow = 0;
69+
if (error == kErrorOk) {
70+
allow = mode->allow[0];
71+
if (allow & kDfuAllowUpLoad) {
72+
// DFU upload means send to the host. We stage the data that would
73+
// be sent to the rescue buffer.
74+
rescue_send_handler(&ctx->state);
6775
}
6876
}
6977

70-
if (error == kErrorOk && mode->upload) {
71-
// DFU upload means send to the host. We stage the data that would
72-
// be sent to the rescue buffer.
73-
rescue_send_handler(state);
74-
}
7578
// BootSvc and OwnerPage are also recv (from the host) services. Make sure
7679
// we're set up to process a DFU download for those services.
77-
if (mode->mode == kRescueModeBootSvcRsp) {
78-
error2 = rescue_validate_mode(kRescueModeBootSvcReq, state);
79-
} else if (mode->mode == kRescueModeOwnerPage0) {
80-
error2 = rescue_validate_mode(kRescueModeOwnerBlock, state);
80+
if (mode->paired) {
81+
rom_error_t error2 = rescue_validate_mode(mode->paired, &ctx->state);
82+
if (error2 == kErrorOk) {
83+
allow |= mode->allow[1];
84+
}
8185
}
8286

83-
if (error == kErrorOk || error2 == kErrorOk) {
84-
// If either the send or recv mode is ok, then setting the mode is ok.
85-
// If only one is Ok, the send or recv handler will report the error when we
86-
// try an unauthorized operation.
87+
// If either the send or recv mode is ok, then setting the mode is ok.
88+
// If only one is Ok, the send or recv handler will report the error when we
89+
// try an unauthorized operation.
90+
// If neither is Ok, the report bad mode.
91+
if (allow) {
92+
ctx->allow = allow;
8793
return kErrorOk;
94+
} else {
95+
return kErrorRescueBadMode;
8896
}
89-
// If neither is Ok, the report bad mode.
90-
return kErrorRescueBadMode;
9197
}
9298

9399
static rom_error_t dfu_control(dfu_ctx_t *ctx, usb_setup_data_t *setup) {
@@ -125,8 +131,8 @@ static rom_error_t dfu_control(dfu_ctx_t *ctx, usb_setup_data_t *setup) {
125131
ctx->dfu_state = tr->next[setup->length == 0 ? 0 : 1];
126132
// Length is good and the request is either upload/download.
127133
if (setup->length <= sizeof(ctx->state.data) &&
128-
(setup->request == kDfuReqDnLoad ||
129-
setup->request == kDfuReqUpLoad)) {
134+
((setup->request == kDfuReqDnLoad && ctx->allow & kDfuAllowDnLoad) ||
135+
(setup->request == kDfuReqUpLoad && ctx->allow & kDfuAllowUpLoad))) {
130136
if (setup->request == kDfuReqDnLoad) {
131137
if (setup->length == 0 ||
132138
ctx->state.offset < ctx->state.flash_limit) {
@@ -193,7 +199,7 @@ static rom_error_t vendor_request(dfu_ctx_t *ctx, usb_setup_data_t *setup) {
193199
// FourCC from the value and index fields.
194200
case kDfuVendorSetMode: {
195201
uint32_t mode = ((uint32_t)setup->value << 16) | setup->index;
196-
if (validate_mode(mode, &ctx->state) == kErrorOk) {
202+
if (validate_mode(mode, ctx) == kErrorOk) {
197203
dfu_transport_data(ctx, kUsbDirIn, NULL, 0, 0);
198204
} else {
199205
return kErrorUsbBadSetup;
@@ -208,7 +214,7 @@ static rom_error_t vendor_request(dfu_ctx_t *ctx, usb_setup_data_t *setup) {
208214
static rom_error_t interface_request(dfu_ctx_t *ctx, usb_setup_data_t *setup) {
209215
switch (setup->request) {
210216
case kUsbSetupReqSetInterface:
211-
if (validate_mode(setup->value, &ctx->state) == kErrorOk) {
217+
if (validate_mode(setup->value, ctx) == kErrorOk) {
212218
// A typical OS USB stack will issue SET_INTERFACE to activate
213219
// AltSetting 0. Since this probably is not real user activity,
214220
// we won't cancel the inactivity deadline for this action when it
@@ -237,7 +243,7 @@ static rom_error_t set_configuration(dfu_ctx_t *ctx) {
237243
ctx->dfu_error = kDfuErrOk;
238244
ctx->dfu_state = kDfuStateIdle;
239245
ctx->interface = 0;
240-
validate_mode(ctx->interface, &ctx->state);
246+
validate_mode(ctx->interface, ctx);
241247
ctx->ep0.configuration = ctx->ep0.next.configuration;
242248
return kErrorOk;
243249
}

sw/device/silicon_creator/lib/rescue/dfu.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ typedef enum dfu_action_t {
8181
kDfuActionReset,
8282
} dfu_action_t;
8383

84+
typedef enum dfu_allow {
85+
kDfuAllowDnLoad = 1,
86+
kDfuAllowUpLoad = 2,
87+
} dfu_allow_t;
88+
8489
/**
8590
* A DFU state transition.
8691
*
@@ -141,6 +146,8 @@ typedef struct dfu_ctx {
141146
uint8_t dfu_error;
142147
/** Currenty selected usb interface setting. */
143148
uint8_t interface;
149+
/** A `dfu_allow_t` describing whether dnload or upload are allowed. */
150+
uint8_t allow;
144151
} dfu_ctx_t;
145152

146153
/**

0 commit comments

Comments
 (0)