- Kernel - mailweb.openeuler.org

[PATCH kernel-4.19] blk-mq: complete req in softirq context in case of single queue
by Yang Yingliang 26 Oct '21

26 Oct '21

From: Ming Lei <ming.lei(a)redhat.com> mainline inclusion from mainline-4.20-rc1 commit 36e765392e48e0322222347c4d21078c0b94758c category: bugfix bugzilla: 175585 CVE: NA ------------------------------------------------- Lot of controllers may have only one irq vector for completing IO request. And usually affinity of the only irq vector is all possible CPUs, however, on most of ARCH, there may be only one specific CPU for handling this interrupt. So if all IOs are completed in hardirq context, it is inevitable to degrade IO performance because of increased irq latency. This patch tries to address this issue by allowing to complete request in softirq context, like the legacy IO path. IOPS is observed as ~13%+ in the following randread test on raid0 over virtio-scsi. mdadm --create --verbose /dev/md0 --level=0 --chunk=1024 --raid-devices=8 /dev/sdb /dev/sdc /dev/sdd /dev/sde /dev/sdf /dev/sdg /dev/sdh /dev/sdi fio --time_based --name=benchmark --runtime=30 --filename=/dev/md0 --nrfiles=1 --ioengine=libaio --iodepth=32 --direct=1 --invalidate=1 --verify=0 --verify_fatal=0 --numjobs=32 --rw=randread --blocksize=4k Cc: Dongli Zhang <dongli.zhang(a)oracle.com> Cc: Zach Marano <zmarano(a)google.com> Cc: Christoph Hellwig <hch(a)lst.de> Cc: Bart Van Assche <bvanassche(a)acm.org> Cc: Jianchao Wang <jianchao.w.wang(a)oracle.com> Signed-off-by: Ming Lei <ming.lei(a)redhat.com> Signed-off-by: Jens Axboe <axboe(a)kernel.dk> Signed-off-by: Lihong Kou <koulihong(a)huawei.com> Reviewed-by: Tao Hou <houtao1(a)huawei.com> Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> --- block/blk-mq.c | 14 ++++++++++++++ block/blk-softirq.c | 5 ++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index be1e2ad4631aa..52a04f6ffeea2 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -603,6 +603,20 @@ void blk_mq_force_complete_rq(struct request *rq) if (rq->internal_tag != -1) blk_mq_sched_completed_request(rq); + /* + * Most of single queue controllers, there is only one irq vector + * for handling IO completion, and the only irq's affinity is set + * as all possible CPUs. On most of ARCHs, this affinity means the + * irq is handled on one specific CPU. + * + * So complete IO reqeust in softirq context in case of single queue + * for not degrading IO performance by irqsoff latency. + */ + if (rq->q->nr_hw_queues == 1) { + __blk_complete_request(rq); + return; + } + if (!test_bit(QUEUE_FLAG_SAME_COMP, &rq->q->queue_flags)) { rq->q->softirq_done_fn(rq); return; diff --git a/block/blk-softirq.c b/block/blk-softirq.c index 15c1f5e12eb89..e47a2f751884d 100644 --- a/block/blk-softirq.c +++ b/block/blk-softirq.c @@ -97,8 +97,8 @@ static int blk_softirq_cpu_dead(unsigned int cpu) void __blk_complete_request(struct request *req) { - int ccpu, cpu; struct request_queue *q = req->q; + int cpu, ccpu = q->mq_ops ? req->mq_ctx->cpu : req->cpu; unsigned long flags; bool shared = false; @@ -110,8 +110,7 @@ void __blk_complete_request(struct request *req) /* * Select completion CPU */ - if (req->cpu != -1) { - ccpu = req->cpu; + if (test_bit(QUEUE_FLAG_SAME_COMP, &q->queue_flags) && ccpu != -1) { if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags)) shared = cpus_share_cache(cpu, ccpu); } else -- 2.25.1

1 0

[PATCH kernel-4.19 1/8] ovl: simplify setting of origin for index lookup
by Yang Yingliang 26 Oct '21

26 Oct '21

From: Vivek Goyal <vgoyal(a)redhat.com> mainline inclusion from mainline-v5.8-rc1 commit 59fb20138a9b5249a4176d5bbc5c670a97343061 category: bugfix bugzilla: NA CVE: NA ------------------------------------------------- overlayfs can keep index of copied up files and directories and it seems to serve two primary puroposes. For regular files, it avoids breaking lower hardlinks over copy up. For directories it seems to be used for various error checks. During ovl_lookup(), we lookup for index using lower dentry in many a cases. That lower dentry is called "origin" and following is a summary of current logic. If there is no upperdentry, always lookup for index using lower dentry. For regular files it helps avoiding breaking hard links over copyup and for directories it seems to be just error checks. If there is an upperdentry, then there are 3 possible cases. - For directories, lower dentry is found using two ways. One is regular path based lookup in lower layers and second is using ORIGIN xattr on upper dentry. First verify that path based lookup lower dentry matches the one pointed by upper ORIGIN xattr. If yes, use this verified origin for index lookup. - For regular files (non-metacopy), there is no path based lookup in lower layers as lookup stops once we find upper dentry. So there is no origin verification. If there is ORIGIN xattr present on upper, use that to lookup index otherwise don't. - For regular metacopy files, again lower dentry is found using path based lookup as well as ORIGIN xattr on upper. Path based lookup is continued in this case to find lower data dentry for metacopy upper. So like directories we only use verified origin. If ORIGIN xattr is not present (Either because lower did not support file handles or because this is hardlink copied up with index=off), then don't use path lookup based lower dentry as origin. This is same as regular non-metacopy file case. Suggested-by: Amir Goldstein <amir73il(a)gmail.com> Signed-off-by: Vivek Goyal <vgoyal(a)redhat.com> Reviewed-by: Amir Goldstein <amir73il(a)gmail.com> Signed-off-by: Miklos Szeredi <mszeredi(a)redhat.com> Signed-off-by: Zheng Liang <zhengliang6(a)huawei.com> Reviewed-by: Zhang Yi <yi.zhang(a)huawei.com> Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> --- fs/overlayfs/namei.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 145bfdde53feb..968ad757c578e 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -1014,25 +1014,30 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, } stack = origin_path; ctr = 1; + origin = origin_path->dentry; origin_path = NULL; } /* - * Lookup index by lower inode and verify it matches upper inode. - * We only trust dir index if we verified that lower dir matches - * origin, otherwise dir index entries may be inconsistent and we - * ignore them. + * Always lookup index if there is no-upperdentry. * - * For non-dir upper metacopy dentry, we already set "origin" if we - * verified that lower matched upper origin. If upper origin was - * not present (because lower layer did not support fh encode/decode), - * or indexing is not enabled, do not set "origin" and skip looking up - * index. This case should be handled in same way as a non-dir upper - * without ORIGIN is handled. + * For the case of upperdentry, we have set origin by now if it + * needed to be set. There are basically three cases. + * + * For directories, lookup index by lower inode and verify it matches + * upper inode. We only trust dir index if we verified that lower dir + * matches origin, otherwise dir index entries may be inconsistent + * and we ignore them. + * + * For regular upper, we already set origin if upper had ORIGIN + * xattr. There is no verification though as there is no path + * based dentry lookup in lower in this case. + * + * For metacopy upper, we set a verified origin already if index + * is enabled and if upper had an ORIGIN xattr. * - * Always lookup index of non-dir non-metacopy and non-upper. */ - if (ctr && (!upperdentry || (!d.is_dir && !metacopy))) + if (!upperdentry && ctr) origin = stack[0].dentry; if (origin && ovl_indexdir(dentry->d_sb) && -- 2.25.1

1 7

Re: [PATCH openEuler-21.03] USB: f_ncm: ncm_bitrate (speed) is unsigned
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:03, xjx00 写道: > From: Maciej Żenczykowski <maze(a)google.com> > > stable inclusion > from stable-v5.10.44 > commit 0f5a20b1fd9da3ac9f7c6edcad522712ca694d5c > bugzilla:https://bugzilla.openeuler.org/show_bug.cgi?id=358 > CVE: NA > > ------------------------------------------------- > > commit 3370139745853f7826895293e8ac3aec1430508e upstream. > > [ 190.544755] configfs-gadget gadget: notify speed -44967296 > > This is because 4250000000 - 2**32 is -44967296. > > Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added") > Cc: Brooke Basile <brookebasile(a)gmail.com> > Cc: Bryan O'Donoghue <bryan.odonoghue(a)linaro.org> > Cc: Felipe Balbi <balbi(a)kernel.org> > Cc: Lorenzo Colitti <lorenzo(a)google.com> > Cc: Yauheni Kaliuta <yauheni.kaliuta(a)nokia.com> > Cc: Linux USB Mailing List <linux-usb(a)vger.kernel.org> > Acked-By: Lorenzo Colitti <lorenzo(a)google.com> > Signed-off-by: Maciej Żenczykowski <maze(a)google.com> > Cc: stable <stable(a)vger.kernel.org> > Link: https://lore.kernel.org/r/20210608005344.3762668-1-zenczykowski@gmail.com > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: xjx00 <xjxyklwx(a)126.com> > --- > drivers/usb/gadget/function/f_ncm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c > index 019bea8e09cc..0d23c6c11a13 100644 > --- a/drivers/usb/gadget/function/f_ncm.c > +++ b/drivers/usb/gadget/function/f_ncm.c > @@ -583,7 +583,7 @@ static void ncm_do_notify(struct f_ncm *ncm) > data[0] = cpu_to_le32(ncm_bitrate(cdev->gadget)); > data[1] = data[0]; > > - DBG(cdev, "notify speed %d\n", ncm_bitrate(cdev->gadget)); > + DBG(cdev, "notify speed %u\n", ncm_bitrate(cdev->gadget)); > ncm->notify_state = NCM_NOTIFY_CONNECT; > break; > }

2 1

Re: [PATCH openEuler-21.03] ALSA: firewire-lib: fix the context to call snd_pcm_stop_xrun()
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 21:17, lihao 写道: > From: Takashi Sakamoto <o-takashi(a)sakamocchi.jp> > > stable inclusion > from stable-v5.10.44 > commit 98f842951f8aa222e8a8453e6dbce6c056e9984f > bugzilla:https://bugzilla.openeuler.org/show_bug.cgi?id=430 > CVE: NA > > ------------------------------------------------- > > commit 9981b20a5e3694f4625ab5a1ddc98ce7503f6d12 upstream. > > In the workqueue to queue wake-up event, isochronous context is not > processed, thus it's useless to check context for the workqueue to switch > status of runtime for PCM substream to XRUN. On the other hand, in > software IRQ context of 1394 OHCI, it's needed. > > This commit fixes the bug introduced when tasklet was replaced with > workqueue. > > Cc: <stable(a)vger.kernel.org> > Fixes: 2b3d2987d800 ("ALSA: firewire: Replace tasklet with work") > Signed-off-by: Takashi Sakamoto <o-takashi(a)sakamocchi.jp> > Link: https://lore.kernel.org/r/20210605091054.68866-1-o-takashi@sakamocchi.jp > Signed-off-by: Takashi Iwai <tiwai(a)suse.de> > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: lihao <380525608(a)qq.com> > --- > sound/firewire/amdtp-stream.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c > index e0faa6601966..5805c5de39fb 100644 > --- a/sound/firewire/amdtp-stream.c > +++ b/sound/firewire/amdtp-stream.c > @@ -804,7 +804,7 @@ static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs, > static inline void cancel_stream(struct amdtp_stream *s) > { > s->packet_index = -1; > - if (current_work() == &s->period_work) > + if (in_interrupt()) > amdtp_stream_pcm_abort(s); > WRITE_ONCE(s->pcm_buffer_pointer, SNDRV_PCM_POS_XRUN); > }

1 0

Re: [PATCH openEuler-21.03] drm/msm/a6xx: avoid shadow NULL reference in failure path
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:02, lzb 写道: > From: Jonathan Marek <jonathan(a)marek.ca> > > stable inclusion > from stable-v5.10.44 > commit fd681a8c7ac8f649a0718f6cbf2fe75d0587c9a2 > bugzilla: > CVE: NA > > https://bugzilla.openeuler.org/show_bug.cgi?id=472-------------------------… > > commit ce86c239e4d218ae6040bec18e6d19a58edb8b7c upstream. > > If a6xx_hw_init() fails before creating the shadow_bo, the a6xx_pm_suspend > code referencing it will crash. Change the condition to one that avoids > this problem (note: creation of shadow_bo is behind this same condition) > > Fixes: e8b0b994c3a5 ("drm/msm/a6xx: Clear shadow on suspend") > Signed-off-by: Jonathan Marek <jonathan(a)marek.ca> > Reviewed-by: Akhil P Oommen <akhilpo(a)codeaurora.org> > Link: https://lore.kernel.org/r/20210513171431.18632-6-jonathan@marek.ca > Signed-off-by: Rob Clark <robdclark(a)chromium.org> > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: lzb <zbliancs(a)qq.com> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 722c2fe3bfd5..7061ba457c5b 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1055,7 +1055,7 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu) > if (ret) > return ret; > > - if (adreno_gpu->base.hw_apriv || a6xx_gpu->has_whereami) > + if (a6xx_gpu->shadow_bo) > for (i = 0; i < gpu->nr_rings; i++) > a6xx_gpu->shadow[i] = 0; >

1 0

Re: [PATCH openEuler-21.03] sched/fair: Make sure to update tg contrib for blocked load
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:09, wjy 写道: > From: Vincent Guittot <vincent.guittot(a)linaro.org> > > stable inclusion > from stable-v5.10.44 > commit 32e22db8b25ea165bd9e446c7f92b089c8568eaf > bugzilla: https://bugzilla.openeuler.org/show_bug.cgi?id=379 > CVE: NA > > ------------------------------------------------- > > commit 02da26ad5ed6ea8680e5d01f20661439611ed776 upstream. > > During the update of fair blocked load (__update_blocked_fair()), we > update the contribution of the cfs in tg->load_avg if cfs_rq's pelt > has decayed. Nevertheless, the pelt values of a cfs_rq could have > been recently updated while propagating the change of a child. In this > case, cfs_rq's pelt will not decayed because it has already been > updated and we don't update tg->load_avg. > > __update_blocked_fair > ... > for_each_leaf_cfs_rq_safe: child cfs_rq > update cfs_rq_load_avg() for child cfs_rq > ... > update_load_avg(cfs_rq_of(se), se, 0) > ... > update cfs_rq_load_avg() for parent cfs_rq > -propagation of child's load makes parent cfs_rq->load_sum > becoming null > -UPDATE_TG is not set so it doesn't update parent > cfs_rq->tg_load_avg_contrib > .. > for_each_leaf_cfs_rq_safe: parent cfs_rq > update cfs_rq_load_avg() for parent cfs_rq > - nothing to do because parent cfs_rq has already been updated > recently so cfs_rq->tg_load_avg_contrib is not updated > ... > parent cfs_rq is decayed > list_del_leaf_cfs_rq parent cfs_rq > - but it still contibutes to tg->load_avg > > we must set UPDATE_TG flags when propagting pending load to the parent > > Fixes: 039ae8bcf7a5 ("sched/fair: Fix O(nr_cgroups) in the load balancing path") > Reported-by: Odin Ugedal <odin(a)uged.al> > Signed-off-by: Vincent Guittot <vincent.guittot(a)linaro.org> > Signed-off-by: Peter Zijlstra (Intel) <peterz(a)infradead.org> > Reviewed-by: Odin Ugedal <odin(a)uged.al> > Link: https://lkml.kernel.org/r/20210527122916.27683-3-vincent.guittot@linaro.org > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: wjy <464310675(a)qq.com> > --- > kernel/sched/fair.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 1ad0e52487f6..43497d88a330 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -7961,7 +7961,7 @@ static bool __update_blocked_fair(struct rq *rq, bool *done) > /* Propagate pending load changes to the parent, if any: */ > se = cfs_rq->tg->se[cpu]; > if (se && !skip_blocked_update(se)) > - update_load_avg(cfs_rq_of(se), se, 0); > + update_load_avg(cfs_rq_of(se), se, UPDATE_TG); > > /* > * There can be a lot of idle CPU cgroups. Don't let fully

1 0

Re: [PATCH openEuler-21.03] NFSv4: Fix second deadlock in nfs4_evict_inode()
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:13, wyp 写道: > From: Trond Myklebust <trond.myklebust(a)hammerspace.com> > > stable inclusion > from stable-v5.10.44 > commit d973bd0d6e7f9b4ea976cc619e8d6e0d235b9056 > bugzilla: https://bugzilla.openeuler.org/show_bug.cgi?id=462 > CVE: NA > > ------------------------------------------------- > > commit c3aba897c6e67fa464ec02b1f17911577d619713 upstream. > > If the inode is being evicted but has to return a layout first, then > that too can cause a deadlock in the corner case where the server > reboots. > > Signed-off-by: Trond Myklebust <trond.myklebust(a)hammerspace.com> > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> > Signed-off-by: Hang <haihangyiyuan(a)163.com> > Reviewed-by: Jian Cheng <cj.chengjian(a)huawei.com> > Signed-off-by: Wang ShaoBo <bobo.shaobowang(a)huawei.com> > --- > fs/nfs/nfs4proc.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index c92d6ff0fcea..eedcbe6832fb 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -9619,15 +9619,20 @@ int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp, bool sync) > &task_setup_data.rpc_client, &msg); > > dprintk("--> %s\n", __func__); > + lrp->inode = nfs_igrab_and_active(lrp->args.inode); > if (!sync) { > - lrp->inode = nfs_igrab_and_active(lrp->args.inode); > if (!lrp->inode) { > nfs4_layoutreturn_release(lrp); > return -EAGAIN; > } > task_setup_data.flags |= RPC_TASK_ASYNC; > } > - nfs4_init_sequence(&lrp->args.seq_args, &lrp->res.seq_res, 1, 0); > + if (!lrp->inode) > + nfs4_init_sequence(&lrp->args.seq_args, &lrp->res.seq_res, 1, > + 1); > + else > + nfs4_init_sequence(&lrp->args.seq_args, &lrp->res.seq_res, 1, > + 0); > task = rpc_run_task(&task_setup_data); > if (IS_ERR(task)) > return PTR_ERR(task);

1 0

Re: [PATCH openEuler-21.03] net: mdiobus: get rid of a BUG_ON()
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:15, gpj 写道: > From: Dan Carpenter <dan.carpenter(a)oracle.com> > > stable inclusion > from stable-v5.10.44 > commit be23c4af3d8a1b986fe9b43b8966797653a76ca4 > bugzilla: https://bugzilla.openeuler.org/show_bug.cgi?id=341 > CVE: NA > > -------------------------------- > > [ Upstream commit 1dde47a66d4fb181830d6fa000e5ea86907b639e ] > > We spotted a bug recently during a review where a driver was > unregistering a bus that wasn't registered, which would trigger this > BUG_ON(). Let's handle that situation more gracefully, and just print > a warning and return. > > Reported-by: Russell King (Oracle) <rmk+kernel(a)armlinux.org.uk> > Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> > Reviewed-by: Russell King (Oracle) <rmk+kernel(a)armlinux.org.uk> > Reviewed-by: Andrew Lunn <andrew(a)lunn.ch> > Signed-off-by: David S. Miller <davem(a)davemloft.net> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > Signed-off-by: wangqing <wangqing(a)uniontech.com> > Reviewed-by: Xie XiuQi <xiexiuqi(a)huawei.com> > Signed-off-by: Zheng Zengkai <zhengzengkai(a)huawei.com> > --- > drivers/net/phy/mdio_bus.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 757e950fb745..b848439fa837 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -608,7 +608,8 @@ void mdiobus_unregister(struct mii_bus *bus) > struct mdio_device *mdiodev; > int i; > > - BUG_ON(bus->state != MDIOBUS_REGISTERED); > + if (WARN_ON_ONCE(bus->state != MDIOBUS_REGISTERED)) > + return; > bus->state = MDIOBUS_UNREGISTERED; > > for (i = 0; i < PHY_MAX_ADDR; i++) {

1 0

Re: [PATCH openEuler-21.03] net: mdiobus: get rid of a BUG_ON()
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:18, dqh 写道: > From: Dan Carpenter <dan.carpenter(a)oracle.com> > > stable inclusion > from stable-v5.10.44 > commit be23c4af3d8a1b986fe9b43b8966797653a76ca4 > bugzilla: https://bugzilla.openeuler.org/show_bug.cgi?id=341 > CVE: NA > > -------------------------------- > > [ Upstream commit 1dde47a66d4fb181830d6fa000e5ea86907b639e ] > > We spotted a bug recently during a review where a driver was > unregistering a bus that wasn't registered, which would trigger this > BUG_ON(). Let's handle that situation more gracefully, and just print > a warning and return. > > Reported-by: Russell King (Oracle) <rmk+kernel(a)armlinux.org.uk> > Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> > Reviewed-by: Russell King (Oracle) <rmk+kernel(a)armlinux.org.uk> > Reviewed-by: Andrew Lunn <andrew(a)lunn.ch> > Signed-off-by: David S. Miller <davem(a)davemloft.net> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > Signed-off-by: wangqing <wangqing(a)uniontech.com> > Reviewed-by: Xie XiuQi <xiexiuqi(a)huawei.com> > Signed-off-by: Zheng Zengkai <zhengzengkai(a)huawei.com> > --- > drivers/net/phy/mdio_bus.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 757e950fb745..b848439fa837 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -608,7 +608,8 @@ void mdiobus_unregister(struct mii_bus *bus) > struct mdio_device *mdiodev; > int i; > > - BUG_ON(bus->state != MDIOBUS_REGISTERED); > + if (WARN_ON_ONCE(bus->state != MDIOBUS_REGISTERED)) > + return; > bus->state = MDIOBUS_UNREGISTERED; > > for (i = 0; i < PHY_MAX_ADDR; i++) {

1 0

Re: [PATCH openEuler-21.03] spi: Cleanup on failure of initial setup
by chengjian (D) 26 Oct '21

26 Oct '21

Reviewed-by: Cheng Jian <cj.chengjian(a)huawei.com> 在 2021/10/23 17:18, dqh 写道: > From: Lukas Wunner <lukas(a)wunner.de> > > stable inclusion > from stable-v5.10.44 > commit fa05ba61967ad051f5f2b7c4f39d6c56719c9900 > bugzilla: https://bugzilla.openeuler.org/show_bug.cgi?id=470 > CVE: NA > > ------------------------------------------------- > > [ Upstream commit 2ec6f20b33eb4f62ab90bdcd620436c883ec3af6 ] > > Commit c7299fea6769 ("spi: Fix spi device unregister flow") changed the > SPI core's behavior if the ->setup() hook returns an error upon adding > an spi_device: Before, the ->cleanup() hook was invoked to free any > allocations that were made by ->setup(). With the commit, that's no > longer the case, so the ->setup() hook is expected to free the > allocations itself. > > I've identified 5 drivers which depend on the old behavior and am fixing > them up hereinafter: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c > spi-omap2-mcspi.c spi-pxa2xx.c > > Importantly, ->setup() is not only invoked on spi_device *addition*: > It may subsequently be called to *change* SPI parameters. If changing > these SPI parameters fails, freeing memory allocations would be wrong. > That should only be done if the spi_device is finally destroyed. > I am therefore using a bool "initial_setup" in 4 of the affected drivers > to differentiate between the invocation on *adding* the spi_device and > any subsequent invocations: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c > spi-omap2-mcspi.c > > In spi-pxa2xx.c, it seems the ->setup() hook can only fail on spi_device > addition, not any subsequent calls. It therefore doesn't need the bool. > > It's worth noting that 5 other drivers already perform a cleanup if the > ->setup() hook fails. Before c7299fea6769, they caused a double-free > if ->setup() failed on spi_device addition. Since the commit, they're > fine. These drivers are: spi-mpc512x-psc.c spi-pl022.c spi-s3c64xx.c > spi-st-ssc4.c spi-tegra114.c > > (spi-pxa2xx.c also already performs a cleanup, but only in one of > several error paths.) > > Fixes: c7299fea6769 ("spi: Fix spi device unregister flow") > Signed-off-by: Lukas Wunner <lukas(a)wunner.de> > Cc: Saravana Kannan <saravanak(a)google.com> > Acked-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com> # pxa2xx > Link: https://lore.kernel.org/r/f76a0599469f265b69c371538794101fa37b5536.16221493… > Signed-off-by: Mark Brown <broonie(a)kernel.org> > Signed-off-by: Sasha Levin <sashal(a)kernel.org> > Signed-off-by: dqh <1486653795(a)qq.com> > --- > drivers/spi/spi-bitbang.c | 18 ++++++++++++++---- > drivers/spi/spi-fsl-spi.c | 4 ++++ > drivers/spi/spi-omap-uwire.c | 9 ++++++++- > drivers/spi/spi-omap2-mcspi.c | 33 ++++++++++++++++++++------------- > drivers/spi/spi-pxa2xx.c | 9 ++++++++- > 5 files changed, 54 insertions(+), 19 deletions(-) > > diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c > index 1a7352abd878..3d8948a17095 100644 > --- a/drivers/spi/spi-bitbang.c > +++ b/drivers/spi/spi-bitbang.c > @@ -181,6 +181,8 @@ int spi_bitbang_setup(struct spi_device *spi) > { > struct spi_bitbang_cs *cs = spi->controller_state; > struct spi_bitbang *bitbang; > + bool initial_setup = false; > + int retval; > > bitbang = spi_master_get_devdata(spi->master); > > @@ -189,22 +191,30 @@ int spi_bitbang_setup(struct spi_device *spi) > if (!cs) > return -ENOMEM; > spi->controller_state = cs; > + initial_setup = true; > } > > /* per-word shift register access, in hardware or bitbanging */ > cs->txrx_word = bitbang->txrx_word[spi->mode & (SPI_CPOL|SPI_CPHA)]; > - if (!cs->txrx_word) > - return -EINVAL; > + if (!cs->txrx_word) { > + retval = -EINVAL; > + goto err_free; > + } > > if (bitbang->setup_transfer) { > - int retval = bitbang->setup_transfer(spi, NULL); > + retval = bitbang->setup_transfer(spi, NULL); > if (retval < 0) > - return retval; > + goto err_free; > } > > dev_dbg(&spi->dev, "%s, %u nsec/bit\n", __func__, 2 * cs->nsecs); > > return 0; > + > +err_free: > + if (initial_setup) > + kfree(cs); > + return retval; > } > EXPORT_SYMBOL_GPL(spi_bitbang_setup); > > diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c > index d0e5aa18b7ba..bdf94cc7be1a 100644 > --- a/drivers/spi/spi-fsl-spi.c > +++ b/drivers/spi/spi-fsl-spi.c > @@ -440,6 +440,7 @@ static int fsl_spi_setup(struct spi_device *spi) > { > struct mpc8xxx_spi *mpc8xxx_spi; > struct fsl_spi_reg __iomem *reg_base; > + bool initial_setup = false; > int retval; > u32 hw_mode; > struct spi_mpc8xxx_cs *cs = spi_get_ctldata(spi); > @@ -452,6 +453,7 @@ static int fsl_spi_setup(struct spi_device *spi) > if (!cs) > return -ENOMEM; > spi_set_ctldata(spi, cs); > + initial_setup = true; > } > mpc8xxx_spi = spi_master_get_devdata(spi->master); > > @@ -475,6 +477,8 @@ static int fsl_spi_setup(struct spi_device *spi) > retval = fsl_spi_setup_transfer(spi, NULL); > if (retval < 0) { > cs->hw_mode = hw_mode; /* Restore settings */ > + if (initial_setup) > + kfree(cs); > return retval; > } > > diff --git a/drivers/spi/spi-omap-uwire.c b/drivers/spi/spi-omap-uwire.c > index 71402f71ddd8..df28c6664aba 100644 > --- a/drivers/spi/spi-omap-uwire.c > +++ b/drivers/spi/spi-omap-uwire.c > @@ -424,15 +424,22 @@ static int uwire_setup_transfer(struct spi_device *spi, struct spi_transfer *t) > static int uwire_setup(struct spi_device *spi) > { > struct uwire_state *ust = spi->controller_state; > + bool initial_setup = false; > + int status; > > if (ust == NULL) { > ust = kzalloc(sizeof(*ust), GFP_KERNEL); > if (ust == NULL) > return -ENOMEM; > spi->controller_state = ust; > + initial_setup = true; > } > > - return uwire_setup_transfer(spi, NULL); > + status = uwire_setup_transfer(spi, NULL); > + if (status && initial_setup) > + kfree(ust); > + > + return status; > } > > static void uwire_cleanup(struct spi_device *spi) > diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c > index d4c9510af393..3596bbe4b776 100644 > --- a/drivers/spi/spi-omap2-mcspi.c > +++ b/drivers/spi/spi-omap2-mcspi.c > @@ -1032,8 +1032,22 @@ static void omap2_mcspi_release_dma(struct spi_master *master) > } > } > > +static void omap2_mcspi_cleanup(struct spi_device *spi) > +{ > + struct omap2_mcspi_cs *cs; > + > + if (spi->controller_state) { > + /* Unlink controller state from context save list */ > + cs = spi->controller_state; > + list_del(&cs->node); > + > + kfree(cs); > + } > +} > + > static int omap2_mcspi_setup(struct spi_device *spi) > { > + bool initial_setup = false; > int ret; > struct omap2_mcspi *mcspi = spi_master_get_devdata(spi->master); > struct omap2_mcspi_regs *ctx = &mcspi->ctx; > @@ -1051,35 +1065,28 @@ static int omap2_mcspi_setup(struct spi_device *spi) > spi->controller_state = cs; > /* Link this to context save list */ > list_add_tail(&cs->node, &ctx->cs); > + initial_setup = true; > } > > ret = pm_runtime_get_sync(mcspi->dev); > if (ret < 0) { > pm_runtime_put_noidle(mcspi->dev); > + if (initial_setup) > + omap2_mcspi_cleanup(spi); > > return ret; > } > > ret = omap2_mcspi_setup_transfer(spi, NULL); > + if (ret && initial_setup) > + omap2_mcspi_cleanup(spi); > + > pm_runtime_mark_last_busy(mcspi->dev); > pm_runtime_put_autosuspend(mcspi->dev); > > return ret; > } > > -static void omap2_mcspi_cleanup(struct spi_device *spi) > -{ > - struct omap2_mcspi_cs *cs; > - > - if (spi->controller_state) { > - /* Unlink controller state from context save list */ > - cs = spi->controller_state; > - list_del(&cs->node); > - > - kfree(cs); > - } > -} > - > static irqreturn_t omap2_mcspi_irq_handler(int irq, void *data) > { > struct omap2_mcspi *mcspi = data; > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > index d6b534d38e5d..56a62095ec8c 100644 > --- a/drivers/spi/spi-pxa2xx.c > +++ b/drivers/spi/spi-pxa2xx.c > @@ -1254,6 +1254,8 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip, > chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH; > > err = gpiod_direction_output(gpiod, !chip->gpio_cs_inverted); > + if (err) > + gpiod_put(chip->gpiod_cs); > } > > return err; > @@ -1267,6 +1269,7 @@ static int setup(struct spi_device *spi) > struct driver_data *drv_data = > spi_controller_get_devdata(spi->controller); > uint tx_thres, tx_hi_thres, rx_thres; > + int err; > > switch (drv_data->ssp_type) { > case QUARK_X1000_SSP: > @@ -1413,7 +1416,11 @@ static int setup(struct spi_device *spi) > if (drv_data->ssp_type == CE4100_SSP) > return 0; > > - return setup_cs(spi, chip, chip_info); > + err = setup_cs(spi, chip, chip_info); > + if (err) > + kfree(chip); > + > + return err; > } > > static void cleanup(struct spi_device *spi)

1 0