Skip to content

Commit

Permalink
USB: usb-storage: Fix use of bitfields for hardware data in ene_ub6250.c
Browse files Browse the repository at this point in the history 
The kernel test robot found a problem with the ene_ub6250 subdriver in
usb-storage: It uses structures containing bitfields to represent
hardware bits in its SD_STATUS, MS_STATUS, and SM_STATUS bytes.  This
is not safe; it presumes a particular bit ordering and it assumes the
compiler will not insert padding, neither of which is guaranteed.

This patch fixes the problem by changing the structures to simple u8
values, with the bitfields replaced by bitmask constants.

CC: <stable@vger.kernel.org>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/YjOcbuU106UpJ/V8@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Alan Stern authored and Greg Kroah-Hartman committed Mar 18, 2022
1 parent 6653b82 commit 1892bf9
Showing 1 changed file with 76 additions and 79 deletions.
155 changes: 76 additions & 79 deletions drivers/usb/storage/ene_ub6250.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,36 +237,33 @@ static struct us_unusual_dev ene_ub6250_unusual_dev_list[] = {
#define memstick_logaddr(logadr1, logadr0) ((((u16)(logadr1)) << 8) | (logadr0))


struct SD_STATUS {
u8 Insert:1;
u8 Ready:1;
u8 MediaChange:1;
u8 IsMMC:1;
u8 HiCapacity:1;
u8 HiSpeed:1;
u8 WtP:1;
u8 Reserved:1;
};

struct MS_STATUS {
u8 Insert:1;
u8 Ready:1;
u8 MediaChange:1;
u8 IsMSPro:1;
u8 IsMSPHG:1;
u8 Reserved1:1;
u8 WtP:1;
u8 Reserved2:1;
};

struct SM_STATUS {
u8 Insert:1;
u8 Ready:1;
u8 MediaChange:1;
u8 Reserved:3;
u8 WtP:1;
u8 IsMS:1;
};
/* SD_STATUS bits */
#define SD_Insert BIT(0)
#define SD_Ready BIT(1)
#define SD_MediaChange BIT(2)
#define SD_IsMMC BIT(3)
#define SD_HiCapacity BIT(4)
#define SD_HiSpeed BIT(5)
#define SD_WtP BIT(6)
/* Bit 7 reserved */

/* MS_STATUS bits */
#define MS_Insert BIT(0)
#define MS_Ready BIT(1)
#define MS_MediaChange BIT(2)
#define MS_IsMSPro BIT(3)
#define MS_IsMSPHG BIT(4)
/* Bit 5 reserved */
#define MS_WtP BIT(6)
/* Bit 7 reserved */

/* SM_STATUS bits */
#define SM_Insert BIT(0)
#define SM_Ready BIT(1)
#define SM_MediaChange BIT(2)
/* Bits 3-5 reserved */
#define SM_WtP BIT(6)
#define SM_IsMS BIT(7)

struct ms_bootblock_cis {
u8 bCistplDEVICE[6]; /* 0 */
Expand Down Expand Up @@ -437,9 +434,9 @@ struct ene_ub6250_info {
u8 *bbuf;

/* for 6250 code */
struct SD_STATUS SD_Status;
struct MS_STATUS MS_Status;
struct SM_STATUS SM_Status;
u8 SD_Status;
u8 MS_Status;
u8 SM_Status;

/* ----- SD Control Data ---------------- */
/*SD_REGISTER SD_Regs; */
Expand Down Expand Up @@ -602,7 +599,7 @@ static int sd_scsi_test_unit_ready(struct us_data *us, struct scsi_cmnd *srb)
{
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;

if (info->SD_Status.Insert && info->SD_Status.Ready)
if ((info->SD_Status & SD_Insert) && (info->SD_Status & SD_Ready))
return USB_STOR_TRANSPORT_GOOD;
else {
ene_sd_init(us);
Expand All @@ -622,7 +619,7 @@ static int sd_scsi_mode_sense(struct us_data *us, struct scsi_cmnd *srb)
0x0b, 0x00, 0x80, 0x08, 0x00, 0x00,
0x71, 0xc0, 0x00, 0x00, 0x02, 0x00 };

if (info->SD_Status.WtP)
if (info->SD_Status & SD_WtP)
usb_stor_set_xfer_buf(mediaWP, 12, srb);
else
usb_stor_set_xfer_buf(mediaNoWP, 12, srb);
Expand All @@ -641,9 +638,9 @@ static int sd_scsi_read_capacity(struct us_data *us, struct scsi_cmnd *srb)
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;

usb_stor_dbg(us, "sd_scsi_read_capacity\n");
if (info->SD_Status.HiCapacity) {
if (info->SD_Status & SD_HiCapacity) {
bl_len = 0x200;
if (info->SD_Status.IsMMC)
if (info->SD_Status & SD_IsMMC)
bl_num = info->HC_C_SIZE-1;
else
bl_num = (info->HC_C_SIZE + 1) * 1024 - 1;
Expand Down Expand Up @@ -693,7 +690,7 @@ static int sd_scsi_read(struct us_data *us, struct scsi_cmnd *srb)
return USB_STOR_TRANSPORT_ERROR;
}

if (info->SD_Status.HiCapacity)
if (info->SD_Status & SD_HiCapacity)
bnByte = bn;

/* set up the command wrapper */
Expand Down Expand Up @@ -733,7 +730,7 @@ static int sd_scsi_write(struct us_data *us, struct scsi_cmnd *srb)
return USB_STOR_TRANSPORT_ERROR;
}

if (info->SD_Status.HiCapacity)
if (info->SD_Status & SD_HiCapacity)
bnByte = bn;

/* set up the command wrapper */
Expand Down Expand Up @@ -1456,7 +1453,7 @@ static int ms_scsi_test_unit_ready(struct us_data *us, struct scsi_cmnd *srb)
struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);

/* pr_info("MS_SCSI_Test_Unit_Ready\n"); */
if (info->MS_Status.Insert && info->MS_Status.Ready) {
if ((info->MS_Status & MS_Insert) && (info->MS_Status & MS_Ready)) {
return USB_STOR_TRANSPORT_GOOD;
} else {
ene_ms_init(us);
Expand All @@ -1476,7 +1473,7 @@ static int ms_scsi_mode_sense(struct us_data *us, struct scsi_cmnd *srb)
0x0b, 0x00, 0x80, 0x08, 0x00, 0x00,
0x71, 0xc0, 0x00, 0x00, 0x02, 0x00 };

if (info->MS_Status.WtP)
if (info->MS_Status & MS_WtP)
usb_stor_set_xfer_buf(mediaWP, 12, srb);
else
usb_stor_set_xfer_buf(mediaNoWP, 12, srb);
Expand All @@ -1495,7 +1492,7 @@ static int ms_scsi_read_capacity(struct us_data *us, struct scsi_cmnd *srb)

usb_stor_dbg(us, "ms_scsi_read_capacity\n");
bl_len = 0x200;
if (info->MS_Status.IsMSPro)
if (info->MS_Status & MS_IsMSPro)
bl_num = info->MSP_TotalBlock - 1;
else
bl_num = info->MS_Lib.NumberOfLogBlock * info->MS_Lib.blockSize * 2 - 1;
Expand Down Expand Up @@ -1650,7 +1647,7 @@ static int ms_scsi_read(struct us_data *us, struct scsi_cmnd *srb)
if (bn > info->bl_num)
return USB_STOR_TRANSPORT_ERROR;

if (info->MS_Status.IsMSPro) {
if (info->MS_Status & MS_IsMSPro) {
result = ene_load_bincode(us, MSP_RW_PATTERN);
if (result != USB_STOR_XFER_GOOD) {
usb_stor_dbg(us, "Load MPS RW pattern Fail !!\n");
Expand Down Expand Up @@ -1751,7 +1748,7 @@ static int ms_scsi_write(struct us_data *us, struct scsi_cmnd *srb)
if (bn > info->bl_num)
return USB_STOR_TRANSPORT_ERROR;

if (info->MS_Status.IsMSPro) {
if (info->MS_Status & MS_IsMSPro) {
result = ene_load_bincode(us, MSP_RW_PATTERN);
if (result != USB_STOR_XFER_GOOD) {
pr_info("Load MSP RW pattern Fail !!\n");
Expand Down Expand Up @@ -1859,12 +1856,12 @@ static int ene_get_card_status(struct us_data *us, u8 *buf)

tmpreg = (u16) reg4b;
reg4b = *(u32 *)(&buf[0x14]);
if (info->SD_Status.HiCapacity && !info->SD_Status.IsMMC)
if ((info->SD_Status & SD_HiCapacity) && !(info->SD_Status & SD_IsMMC))
info->HC_C_SIZE = (reg4b >> 8) & 0x3fffff;

info->SD_C_SIZE = ((tmpreg & 0x03) << 10) | (u16)(reg4b >> 22);
info->SD_C_SIZE_MULT = (u8)(reg4b >> 7) & 0x07;
if (info->SD_Status.HiCapacity && info->SD_Status.IsMMC)
if ((info->SD_Status & SD_HiCapacity) && (info->SD_Status & SD_IsMMC))
info->HC_C_SIZE = *(u32 *)(&buf[0x100]);

if (info->SD_READ_BL_LEN > SD_BLOCK_LEN) {
Expand Down Expand Up @@ -2076,6 +2073,7 @@ static int ene_ms_init(struct us_data *us)
u16 MSP_BlockSize, MSP_UserAreaBlocks;
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
u8 *bbuf = info->bbuf;
unsigned int s;

printk(KERN_INFO "transport --- ENE_MSInit\n");

Expand All @@ -2100,15 +2098,16 @@ static int ene_ms_init(struct us_data *us)
return USB_STOR_TRANSPORT_ERROR;
}
/* the same part to test ENE */
info->MS_Status = *(struct MS_STATUS *) bbuf;

if (info->MS_Status.Insert && info->MS_Status.Ready) {
printk(KERN_INFO "Insert = %x\n", info->MS_Status.Insert);
printk(KERN_INFO "Ready = %x\n", info->MS_Status.Ready);
printk(KERN_INFO "IsMSPro = %x\n", info->MS_Status.IsMSPro);
printk(KERN_INFO "IsMSPHG = %x\n", info->MS_Status.IsMSPHG);
printk(KERN_INFO "WtP= %x\n", info->MS_Status.WtP);
if (info->MS_Status.IsMSPro) {
info->MS_Status = bbuf[0];

s = info->MS_Status;
if ((s & MS_Insert) && (s & MS_Ready)) {
printk(KERN_INFO "Insert = %x\n", !!(s & MS_Insert));
printk(KERN_INFO "Ready = %x\n", !!(s & MS_Ready));
printk(KERN_INFO "IsMSPro = %x\n", !!(s & MS_IsMSPro));
printk(KERN_INFO "IsMSPHG = %x\n", !!(s & MS_IsMSPHG));
printk(KERN_INFO "WtP= %x\n", !!(s & MS_WtP));
if (s & MS_IsMSPro) {
MSP_BlockSize = (bbuf[6] << 8) | bbuf[7];
MSP_UserAreaBlocks = (bbuf[10] << 8) | bbuf[11];
info->MSP_TotalBlock = MSP_BlockSize * MSP_UserAreaBlocks;
Expand Down Expand Up @@ -2169,17 +2168,17 @@ static int ene_sd_init(struct us_data *us)
return USB_STOR_TRANSPORT_ERROR;
}

info->SD_Status = *(struct SD_STATUS *) bbuf;
if (info->SD_Status.Insert && info->SD_Status.Ready) {
struct SD_STATUS *s = &info->SD_Status;
info->SD_Status = bbuf[0];
if ((info->SD_Status & SD_Insert) && (info->SD_Status & SD_Ready)) {
unsigned int s = info->SD_Status;

ene_get_card_status(us, bbuf);
usb_stor_dbg(us, "Insert = %x\n", s->Insert);
usb_stor_dbg(us, "Ready = %x\n", s->Ready);
usb_stor_dbg(us, "IsMMC = %x\n", s->IsMMC);
usb_stor_dbg(us, "HiCapacity = %x\n", s->HiCapacity);
usb_stor_dbg(us, "HiSpeed = %x\n", s->HiSpeed);
usb_stor_dbg(us, "WtP = %x\n", s->WtP);
usb_stor_dbg(us, "Insert = %x\n", !!(s & SD_Insert));
usb_stor_dbg(us, "Ready = %x\n", !!(s & SD_Ready));
usb_stor_dbg(us, "IsMMC = %x\n", !!(s & SD_IsMMC));
usb_stor_dbg(us, "HiCapacity = %x\n", !!(s & SD_HiCapacity));
usb_stor_dbg(us, "HiSpeed = %x\n", !!(s & SD_HiSpeed));
usb_stor_dbg(us, "WtP = %x\n", !!(s & SD_WtP));
} else {
usb_stor_dbg(us, "SD Card Not Ready --- %x\n", bbuf[0]);
return USB_STOR_TRANSPORT_ERROR;
Expand All @@ -2201,14 +2200,14 @@ static int ene_init(struct us_data *us)

misc_reg03 = bbuf[0];
if (misc_reg03 & 0x01) {
if (!info->SD_Status.Ready) {
if (!(info->SD_Status & SD_Ready)) {
result = ene_sd_init(us);
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
}
}
if (misc_reg03 & 0x02) {
if (!info->MS_Status.Ready) {
if (!(info->MS_Status & MS_Ready)) {
result = ene_ms_init(us);
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
Expand Down Expand Up @@ -2307,14 +2306,14 @@ static int ene_transport(struct scsi_cmnd *srb, struct us_data *us)

/*US_DEBUG(usb_stor_show_command(us, srb)); */
scsi_set_resid(srb, 0);
if (unlikely(!(info->SD_Status.Ready || info->MS_Status.Ready)))
if (unlikely(!(info->SD_Status & SD_Ready) || (info->MS_Status & MS_Ready)))
result = ene_init(us);
if (result == USB_STOR_XFER_GOOD) {
result = USB_STOR_TRANSPORT_ERROR;
if (info->SD_Status.Ready)
if (info->SD_Status & SD_Ready)
result = sd_scsi_irp(us, srb);

if (info->MS_Status.Ready)
if (info->MS_Status & MS_Ready)
result = ms_scsi_irp(us, srb);
}
return result;
Expand Down Expand Up @@ -2378,7 +2377,6 @@ static int ene_ub6250_probe(struct usb_interface *intf,

static int ene_ub6250_resume(struct usb_interface *iface)
{
u8 tmp = 0;
struct us_data *us = usb_get_intfdata(iface);
struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);

Expand All @@ -2390,17 +2388,16 @@ static int ene_ub6250_resume(struct usb_interface *iface)
mutex_unlock(&us->dev_mutex);

info->Power_IsResum = true;
/*info->SD_Status.Ready = 0; */
info->SD_Status = *(struct SD_STATUS *)&tmp;
info->MS_Status = *(struct MS_STATUS *)&tmp;
info->SM_Status = *(struct SM_STATUS *)&tmp;
/* info->SD_Status &= ~SD_Ready; */
info->SD_Status = 0;
info->MS_Status = 0;
info->SM_Status = 0;

return 0;
}

static int ene_ub6250_reset_resume(struct usb_interface *iface)
{
u8 tmp = 0;
struct us_data *us = usb_get_intfdata(iface);
struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);

Expand All @@ -2412,10 +2409,10 @@ static int ene_ub6250_reset_resume(struct usb_interface *iface)
* the device
*/
info->Power_IsResum = true;
/*info->SD_Status.Ready = 0; */
info->SD_Status = *(struct SD_STATUS *)&tmp;
info->MS_Status = *(struct MS_STATUS *)&tmp;
info->SM_Status = *(struct SM_STATUS *)&tmp;
/* info->SD_Status &= ~SD_Ready; */
info->SD_Status = 0;
info->MS_Status = 0;
info->SM_Status = 0;

return 0;
}
Expand Down

0 comments on commit 1892bf9

Please sign in to comment.