| Message ID | 1435018875-22527-14-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 22, 2015 at 08:21:12PM -0400, John Snow wrote: > +static AHCICmdHdr *get_cmd_header(AHCIState *s, uint8_t port, uint8_t slot) > +{ > + if (port > s->ports || slot > AHCI_MAX_CMDS) { Should these be >= instead of >? Otherwise 1 element beyond the end of the array can be accessed.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 06/26/2015 11:51 AM, Stefan Hajnoczi wrote: > On Mon, Jun 22, 2015 at 08:21:12PM -0400, John Snow wrote: >> +static AHCICmdHdr *get_cmd_header(AHCIState *s, uint8_t port, >> uint8_t slot) +{ + if (port > s->ports || slot > >> AHCI_MAX_CMDS) { > > Should these be >= instead of >? Otherwise 1 element beyond the > end of the array can be accessed. > Sigh. Yes, yes it should. Thank you for saving me from myself. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJVjZrRAAoJEH3vgQaq/DkOjcMP/0CqxJAWcgsnFH5Opwz7iNbm OyvLakvzUmU958qN3nzG2vMME3WwyVF3bxVJkoT3v1Pc6Tm9e+hq+R1oDzD0TD6X tfjQTc5JZcViKSD74Huo85gKoVp3Tp4idOaaVFn36TmP9gk5i6g/C0Nf7EzY12Er 4op6mLvBhiU6NH72UwP4HHij0KZCMCVYYH4qhAWsil/QIugAgELlM9TFkVI49lHs WN3IE+5gQijd99Z5VJaC9wrmtOdZWHblfTeWMvVQfMfbg5dJ2Aiya1tHlw6QrPzb 7nGPHTwyGLmNsvUsNqMHrWGaL9e4iEwXGYNRnIT16L8QPzGqg7v4D3lvJcGv5LrM OFPje2Fk9w8OGdgRjrQhQcJXfU+IPkvlwnW7G3NO2Ts0GK5cHClRafZVjvAhMuXs 5M89BTdi7LBGJ7EWq8ByP5mxPj+hiR3hsSG41OPFg3e0VpwtgFJ6umaZde1tMrHp IwnOZcvXUoTOAna3Y4E/fSwK1jKczTXIvkl4+HnJB40ekyoSkyl6qXV1FlMRiMnq pAeJ5jgBovuV2gx8pLYU3ipR0LJTjzz7N39FkAJJyekWWjNVrZ4Ue5IGW/pXKVkN SjeSrZTtxSmyO8BPya7/xsqjiqo3v7m2jSBLTVEsUtkFEaet4xlAH2OQZBvlTbuI n5Hc61NkB6qMU5k6We31 =Sfdd -----END PGP SIGNATURE-----
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a29cf49..eeb48fb 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1115,11 +1115,20 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, execute_ncq_command(ncq_tfs); } +static AHCICmdHdr *get_cmd_header(AHCIState *s, uint8_t port, uint8_t slot) +{ + if (port > s->ports || slot > AHCI_MAX_CMDS) { + return NULL; + } + + return s->dev[port].lst ? &((AHCICmdHdr *)s->dev[port].lst)[slot] : NULL; +} + static void handle_reg_h2d_fis(AHCIState *s, int port, uint8_t slot, uint8_t *cmd_fis) { IDEState *ide_state = &s->dev[port].port.ifs[0]; - AHCICmdHdr *cmd = s->dev[port].cur_cmd; + AHCICmdHdr *cmd = get_cmd_header(s, port, slot); uint16_t opts = le16_to_cpu(cmd->opts); if (cmd_fis[1] & 0x0F) { @@ -1218,7 +1227,7 @@ static int handle_cmd(AHCIState *s, int port, uint8_t slot) DPRINTF(port, "error: lst not given but cmd handled"); return -1; } - cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot]; + cmd = get_cmd_header(s, port, slot); /* remember current slot handle for later */ s->dev[port].cur_cmd = cmd; @@ -1592,7 +1601,7 @@ static int ahci_state_post_load(void *opaque, int version_id) if (ncq_tfs->slot > AHCI_MAX_CMDS) { return -1; } - ncq_tfs->cmdh = &((AHCICmdHdr *)ad->lst)[ncq_tfs->slot]; + ncq_tfs->cmdh = get_cmd_header(s, i, ncq_tfs->slot); ahci_populate_sglist(ncq_tfs->drive, &ncq_tfs->sglist, ncq_tfs->cmdh, ncq_tfs->sector_count * 512, 0); @@ -1618,7 +1627,7 @@ static int ahci_state_post_load(void *opaque, int version_id) if (ad->busy_slot < 0 || ad->busy_slot >= AHCI_MAX_CMDS) { return -1; } - ad->cur_cmd = &((AHCICmdHdr *)ad->lst)[ad->busy_slot]; + ad->cur_cmd = get_cmd_header(s, i, ad->busy_slot); } }
cur_cmd is an internal bookmark that points to the current AHCI Command Header being processed by the AHCI state machine. With NCQ needing to occasionally rely on some of the same AHCI helpers, we cannot use cur_cmd and will need to grab explicit pointers instead. In an attempt to begin relying on the cur_cmd pointer less, add a helper to let us specifically get the pointer to the command header of particular interest. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/ide/ahci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)