davidker / unisys

Master repository for new changes to drivers/staging/unisys and drivers/visorbus
Other
2 stars 1 forks source link

Don't check for more than PAGE_SIZE length #108

Closed ghost closed 7 years ago

ghost commented 7 years ago

KanBoard-24515

 >  pscan = ctx->curr;
 >  nscan = ctx->bytes_remaining;
 >  if (nscan == 0)
 >      return NULL;
 >  if (!pscan)
 >      return NULL;
 >  for (i = 0, value_length = -1; i < nscan; i++)
 >      if (pscan[i] == '\0') {
 >          value_length = i;
 >          break;
 >      }
 >  if (value_length < 0)   /* '\0' was not included in the length */
 >      value_length = nscan;
 >  value = kmalloc(value_length + 1, GFP_KERNEL | __GFP_NORETRY);
 >  if (!value)
 >      return NULL;
 >  if (value_length > 0)
 >      memcpy(value, pscan, value_length);
 >  ((u8 *)(value))[value_length] = '\0';
 >  return value;
 > }
 > 
 > static ssize_t toolaction_show(struct device *dev,
 >                 struct device_attribute *attr,
 >                 char *buf)
 > {
 >  u8 tool_action = 0;
 > 
 >  visorchannel_read(controlvm_channel,
 >            offsetof(struct spar_controlvm_channel_protocol,
 >                 tool_action), &tool_action, sizeof(u8));
 >  return scnprintf(buf, PAGE_SIZE, "%u\n", tool_action);

 return sprintf(buf, "%u\n", tool_action);

You can't ever be larger than a page, no need to pretend you could be.

 > }
 > 
 > static ssize_t toolaction_store(struct device *dev,
 >              struct device_attribute *attr,
 >              const char *buf, size_t count)
 > {
 >  u8 tool_action;
 >  int ret;
 > 
 >  if (kstrtou8(buf, 10, &tool_action))
 >      return -EINVAL;
 > 
 >  ret = visorchannel_write
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            tool_action),
 >       &tool_action, sizeof(u8));
 > 
 >  if (ret)
 >      return ret;
 >  return count;
 > }
 > 
 > static ssize_t boottotool_show(struct device *dev,
 >                 struct device_attribute *attr,
 >                 char *buf)
 > {
 >  struct efi_spar_indication efi_spar_indication;
 > 
 >  visorchannel_read(controlvm_channel,
 >            offsetof(struct spar_controlvm_channel_protocol,
 >                 efi_spar_ind), &efi_spar_indication,
 >            sizeof(struct efi_spar_indication));
 >  return scnprintf(buf, PAGE_SIZE, "%u\n",
 >           efi_spar_indication.boot_to_tool);

same here with scnprintf. Everywhere else too, I'm not going to point them all out.

 > }
 > 
 > static ssize_t boottotool_store(struct device *dev,
 >              struct device_attribute *attr,
 >              const char *buf, size_t count)
 > {
 >  int val, ret;
 >  struct efi_spar_indication efi_spar_indication;
 > 
 >  if (kstrtoint(buf, 10, &val))
 >      return -EINVAL;
 > 
 >  efi_spar_indication.boot_to_tool = val;
 >  ret = visorchannel_write
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            efi_spar_ind), &(efi_spar_indication),
 >       sizeof(struct efi_spar_indication));
 > 
 >  if (ret)
 >      return ret;
 >  return count;
 > }
 > 
 > static ssize_t error_show(struct device *dev, struct device_attribute *attr,
 >            char *buf)
 > {
 >  u32 error = 0;
 > 
 >  visorchannel_read(controlvm_channel,
 >            offsetof(struct spar_controlvm_channel_protocol,
 >                 installation_error),
 >            &error, sizeof(u32));
 >  return scnprintf(buf, PAGE_SIZE, "%i\n", error);
 > }
 > 
 > static ssize_t error_store(struct device *dev, struct device_attribute *attr,
 >             const char *buf, size_t count)
 > {
 >  u32 error;
 >  int ret;
 > 
 >  if (kstrtou32(buf, 10, &error))
 >      return -EINVAL;
 > 
 >  ret = visorchannel_write
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            installation_error),
 >       &error, sizeof(u32));
 >  if (ret)
 >      return ret;
 >  return count;
 > }
 > 
 > static ssize_t textid_show(struct device *dev, struct device_attribute *attr,
 >             char *buf)
 > {
 >  u32 text_id = 0;
 > 
 >  visorchannel_read
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            installation_text_id),
 >       &text_id, sizeof(u32));
 >  return scnprintf(buf, PAGE_SIZE, "%i\n", text_id);
 > }
 > 
 > static ssize_t textid_store(struct device *dev, struct device_attribute *attr,
 >              const char *buf, size_t count)
 > {
 >  u32 text_id;
 >  int ret;
 > 
 >  if (kstrtou32(buf, 10, &text_id))
 >      return -EINVAL;
 > 
 >  ret = visorchannel_write
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            installation_text_id),
 >       &text_id, sizeof(u32));
 >  if (ret)
 >      return ret;
 >  return count;
 > }
 > 
 > static ssize_t remaining_steps_show(struct device *dev,
 >                  struct device_attribute *attr, char *buf)
 > {
 >  u16 remaining_steps = 0;
 > 
 >  visorchannel_read(controlvm_channel,
 >            offsetof(struct spar_controlvm_channel_protocol,
 >                 installation_remaining_steps),
 >            &remaining_steps, sizeof(u16));
 >  return scnprintf(buf, PAGE_SIZE, "%hu\n", remaining_steps);
 > }
 > 
 > static ssize_t remaining_steps_store(struct device *dev,
 >                   struct device_attribute *attr,
 >                   const char *buf, size_t count)
 > {
 >  u16 remaining_steps;
 >  int ret;
 > 
 >  if (kstrtou16(buf, 10, &remaining_steps))
 >      return -EINVAL;
 > 
 >  ret = visorchannel_write
 >      (controlvm_channel,
 >       offsetof(struct spar_controlvm_channel_protocol,
 >            installation_remaining_steps),
 >       &remaining_steps, sizeof(u16));
 >  if (ret)
 >      return ret;
 >  return count;
 > }
 > 
 > struct visor_busdev {
 >  u32 bus_no;
 >  u32 dev_no;
 > };
 > 
 > static int match_visorbus_dev_by_id(struct device *dev, void *data)
 > {
 >  struct visor_device *vdev = to_visor_device(dev);
 >  struct visor_busdev *id = data;
 >  u32 bus_no = id->bus_no;
 >  u32 dev_no = id->dev_no;
 > 
 >  if ((vdev->chipset_bus_no == bus_no) &&
 >      (vdev->chipset_dev_no == dev_no))
 >      return 1;
 > 
 >  return 0;
 > }
 > 
 > struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
 >                         struct visor_device *from)
 > {
 >  struct device *dev;
 >  struct device *dev_start = NULL;
 >  struct visor_device *vdev = NULL;
 >  struct visor_busdev id = {
 >          .bus_no = bus_no,
 >          .dev_no = dev_no
 >      };
 > 
 >  if (from)
 >      dev_start = &from->device;
 >  dev = bus_find_device(&visorbus_type, dev_start, (void *)&id,
 >                match_visorbus_dev_by_id);
 >  if (dev)
 >      vdev = to_visor_device(dev);
 >  return vdev;
 > }
 >
ghost commented 7 years ago

Just so we know what we're working with:

$ grep -rn "sc*nprintf" drivers/staging/unisys/*
drivers/staging/unisys/visorbus/visorbus_main.c:52: return snprintf(buf, PAGE_SIZE, "visorbus:%pUl\n", &guid);
drivers/staging/unisys/visorbus/visorbus_main.c:190:    return snprintf(buf, PAGE_SIZE, "0x%llx\n",
drivers/staging/unisys/visorbus/visorbus_main.c:202:    return snprintf(buf, PAGE_SIZE, "0x%lx\n",
drivers/staging/unisys/visorbus/visorbus_main.c:214:    return snprintf(buf, PAGE_SIZE, "0x%llx\n",
drivers/staging/unisys/visorbus/visorbus_main.c:227:    return snprintf(buf, PAGE_SIZE, "%s\n",
drivers/staging/unisys/visorbus/visorbus_main.c:240:    return snprintf(buf, PAGE_SIZE, "%s\n",
drivers/staging/unisys/visorbus/visorbus_main.c:260:    return snprintf(buf, PAGE_SIZE, "%s\n", drv->channel_types[i - 1].name);
drivers/staging/unisys/visorbus/visorbus_main.c:299:    return snprintf(buf, PAGE_SIZE, "0x%llx\n", handle);
drivers/staging/unisys/visorbus/visorbus_main.c:308:    return snprintf(buf, PAGE_SIZE, "{%pUb}\n", &vdev->partition_uuid);
drivers/staging/unisys/visorbus/visorbus_main.c:317:    return snprintf(buf, PAGE_SIZE, "%s\n", vdev->name);
drivers/staging/unisys/visorbus/visorbus_main.c:327:    return snprintf(buf, PAGE_SIZE, "0x%llx\n", addr);
drivers/staging/unisys/visorbus/visorbus_main.c:337:    return snprintf(buf, PAGE_SIZE, "0x%llx\n", nbytes);
drivers/staging/unisys/visorbus/visorchipset.c:191: return scnprintf(buf, PAGE_SIZE, "%u\n", tool_action);
drivers/staging/unisys/visorbus/visorchipset.c:226: return scnprintf(buf, PAGE_SIZE, "%u\n",
drivers/staging/unisys/visorbus/visorchipset.c:262: return scnprintf(buf, PAGE_SIZE, "%i\n", error);
drivers/staging/unisys/visorbus/visorchipset.c:295: return scnprintf(buf, PAGE_SIZE, "%i\n", text_id);
drivers/staging/unisys/visorbus/visorchipset.c:327: return scnprintf(buf, PAGE_SIZE, "%hu\n", remaining_steps);

There are other scnprintf's and snprintf's in our driver code, but the instances above seem to be the only ones that relate to this issue.

ghost commented 7 years ago

Branch: githubissue-108-upstream-next-TurningWrenches Commit(s): bd230937119e60e26b60d3eae702db6c84238114 checkpatch-cleared: Yes T710-1 verified: Yes

See testing results on internal page.

davidker commented 7 years ago

Can you split this into two patches? One for each file? I would like to put the Reported-by on the one for visorchipset, not the one for visorbus.

ghost commented 7 years ago

Branch: githubissue-108-upstream-next-TurningWrenches Commit(s): 3f185e5095c76fb405f25233f139b69c5f45370f 6aa7db3f392170089596e952e525a31ff3322ba8 checkpatch-cleared: Yes T710-W1 verified: Yes, but also pending response from e-mail "Partition Desktop Behavior"

Update: No longer waiting on this e-mail. Issue presents in 3.10 kernel.

davidker commented 7 years ago

This patch was accepted into the upstream. Closing.