Re: [PATCH] lpfc: fixup out-of-bounds access during CPU hotplug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/18/2019 12:21 PM, Hannes Reinecke wrote:
On 11/18/19 8:28 PM, James Smart wrote:
On 11/18/2019 4:30 AM, Hannes Reinecke wrote:
The lpfc driver allocates a cpu_map based on the number of possible
cpus during startup. If a CPU hotplug occurs the number of CPUs
might change, causing an out-of-bounds access when trying to lookup
the hardware index for a given CPU.

Suggested-by: Daniel Wagner <daniel.wagner@xxxxxxxx>
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
---
  drivers/scsi/lpfc/lpfc_scsi.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index ba26df90a36a..2380452a8efd 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -642,7 +642,8 @@ lpfc_get_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp,
      int tag;
      struct fcp_cmd_rsp_buf *tmp = NULL;
-    cpu = raw_smp_processor_id();
+    cpu = min_t(u32, raw_smp_processor_id(),
+            phba->sli4_hba.num_possible_cpu);
      if (cmnd && phba->cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_HDWQ) {
          tag = blk_mq_unique_tag(cmnd->request);
          idx = blk_mq_unique_tag_to_hwq(tag);

This should be unnecessary with the lpfc 12.6.0.1 and 12.6.0.2 patches that tie into cpu onling/offlining and cpu hot add.

I am curious, how if a cpu is hot removed - how num_possible dynamically changes (to a lower value ?) such that a thread can be running on a cpu that returns a higher cpu number than num_possible ?

Actually, the behaviour of num_possible_cpus() under cpu hotplug seems to be implementation-specific. One might want to set it to the max value at all times, which has the drawback that you'd need to scale per-cpu values with that number, leading to a massive memory overhead as only very few installation will ever reach that number. Others might want to set to the max value at the current configuration, implying that it might change under CPU hotplug. Which seems to be the case for PowerPC, which was the case which triggered the issue at hand. But maybe it was a plain bug in the CPU hotplug implementation; one should be asking BenH for details here.

Cheers,

Hannes

That sure seems to be at odds with this comment from include/linux/cpumask.h:

 *  The cpu_possible_mask is fixed at boot time, as the set of CPU id's
 *  that it is possible might ever be plugged in at anytime during the
 *  life of that system boot.  The cpu_present_mask is dynamic(*),
 *  representing which CPUs are currently plugged in.  And
 *  cpu_online_mask is the dynamic subset of cpu_present_mask,
 *  indicating those CPUs available for scheduling.

and
#define num_possible_cpus()     cpumask_weight(cpu_possible_mask)


-- james




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux