[PATCH net-next] virtio_net: Fix error code in __virtnet_get_hw_stats()
The virtnet_send_command_reply() function returns true on success or
false on failure. The "ok" variable is true/false depending on whether
it succeeds or not. It's up to the caller to translate the true/false
into -EINVAL on failure or zero for success.
The bug is that __virtnet_get_hw_stats() returns false for both
errors and success. It's not a bug, but it is confusing that the caller
virtnet_get_hw_stats() uses an "ok" variable to store negative error
codes.
Fix the bug and clean things up so that it's clear that
__virtnet_get_hw_stats() returns zero on success or negative error codes
on failure.
Fixes: 941168f8b40e ("virtio_net: support device stats")
Signed-off-by: Dan Carpenter
---
drivers/net/virtio_net.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 218a446c4c27..4fc0fcdad259 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4016,7 +4016,7 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
_out, _in);
if (!ok)
- return ok;
+ return -EINVAL;
for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
hdr = p;
@@ -4053,7 +4053,7 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
struct virtio_net_ctrl_queue_stats *req;
bool enable_cvq;
void *reply;
- int ok;
+ int err;
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
return 0;
@@ -4100,12 +4100,12 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
if (enable_cvq)
virtnet_make_stat_req(vi, ctx, req, vi->max_queue_pairs * 2,
);
- ok = __virtnet_get_hw_stats(vi, ctx, req, sizeof(*req) * j, reply,
res_size);
+ err = __virtnet_get_hw_stats(vi, ctx, req, sizeof(*req) * j, reply,
res_size);
kfree(req);
kfree(reply);
- return ok;
+ return err;
}
static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8
*data)
Re: [PATCH v2 2/6] iio: light: stk3310: Implement vdd supply and power it off during suspend
Hi Aren,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Aren-Moynihan/dt-bindings-iio-light-stk33xx-add-vdd-and-leda-regulators/20240424-064250
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:
https://lore.kernel.org/r/20240423223309.1468198-4-aren%40peacevolution.org
patch subject: [PATCH v2 2/6] iio: light: stk3310: Implement vdd supply and
power it off during suspend
config: i386-randconfig-141-20240424
(https://download.01.org/0day-ci/archive/20240425/202404251021.4oper3os-...@intel.com/config)
compiler: gcc-10 (Ubuntu 10.5.0-1ubuntu1) 10.5.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202404251021.4oper3os-...@intel.com/
smatch warnings:
drivers/iio/light/stk3310.c:615 stk3310_probe() error: uninitialized symbol
'ret'.
vim +/ret +615 drivers/iio/light/stk3310.c
9046d80dce04c6 Uwe Kleine-König 2022-11-18 592 static int
stk3310_probe(struct i2c_client *client)
be9e6229d67696 Tiberiu Breana 2015-04-27 593 {
be9e6229d67696 Tiberiu Breana 2015-04-27 594 int ret;
be9e6229d67696 Tiberiu Breana 2015-04-27 595 struct iio_dev
*indio_dev;
be9e6229d67696 Tiberiu Breana 2015-04-27 596 struct stk3310_data
*data;
be9e6229d67696 Tiberiu Breana 2015-04-27 597
be9e6229d67696 Tiberiu Breana 2015-04-27 598 indio_dev =
devm_iio_device_alloc(>dev, sizeof(*data));
be9e6229d67696 Tiberiu Breana 2015-04-27 599 if (!indio_dev) {
be9e6229d67696 Tiberiu Breana 2015-04-27 600
dev_err(>dev, "iio allocation failed!\n");
be9e6229d67696 Tiberiu Breana 2015-04-27 601 return -ENOMEM;
be9e6229d67696 Tiberiu Breana 2015-04-27 602 }
be9e6229d67696 Tiberiu Breana 2015-04-27 603
be9e6229d67696 Tiberiu Breana 2015-04-27 604 data =
iio_priv(indio_dev);
be9e6229d67696 Tiberiu Breana 2015-04-27 605 data->client = client;
be9e6229d67696 Tiberiu Breana 2015-04-27 606
i2c_set_clientdata(client, indio_dev);
d6ecb01583d4e0 Arnaud Ferraris 2022-04-20 607
d6ecb01583d4e0 Arnaud Ferraris 2022-04-20 608
device_property_read_u32(>dev, "proximity-near-level",
d6ecb01583d4e0 Arnaud Ferraris 2022-04-20 609
>ps_near_level);
d6ecb01583d4e0 Arnaud Ferraris 2022-04-20 610
be9e6229d67696 Tiberiu Breana 2015-04-27 611 mutex_init(>lock);
be9e6229d67696 Tiberiu Breana 2015-04-27 612
dd231c1d219f6b Ondrej Jirman2024-04-23 613 data->vdd_reg =
devm_regulator_get(>dev, "vdd");
dd231c1d219f6b Ondrej Jirman2024-04-23 614 if
(IS_ERR(data->vdd_reg))
dd231c1d219f6b Ondrej Jirman2024-04-23 @615 return
dev_err_probe(>dev, ret, "get regulator vdd failed\n");
s/ret/PTR_ERR(data->vdd_reg)/
dd231c1d219f6b Ondrej Jirman2024-04-23 616
be9e6229d67696 Tiberiu Breana 2015-04-27 617 ret =
stk3310_regmap_init(data);
be9e6229d67696 Tiberiu Breana 2015-04-27 618 if (ret < 0)
be9e6229d67696 Tiberiu Breana 2015-04-27 619 return ret;
be9e6229d67696 Tiberiu Breana 2015-04-27 620
be9e6229d67696 Tiberiu Breana 2015-04-27 621 indio_dev->info =
_info;
be9e6229d67696 Tiberiu Breana 2015-04-27 622 indio_dev->name =
STK3310_DRIVER_NAME;
be9e6229d67696 Tiberiu Breana 2015-04-27 623 indio_dev->modes =
INDIO_DIRECT_MODE;
be9e6229d67696 Tiberiu Breana 2015-04-27 624 indio_dev->channels =
stk3310_channels;
be9e6229d67696 Tiberiu Breana 2015-04-27 625 indio_dev->num_channels
= ARRAY_SIZE(stk3310_channels);
be9e6229d67696 Tiberiu Breana 2015-04-27 626
dd231c1d219f6b Ondrej Jirman2024-04-23 627 ret =
regulator_enable(data->vdd_reg);
dd231c1d219f6b Ondrej Jirman2024-04-23 628 if (ret)
dd231c1d219f6b Ondrej Jirman2024-04-23 629 return
dev_err_probe(>dev, ret,
dd231c1d219f6b Ondrej Jirman2024-04-23 630
"regulator vdd enable failed\n");
dd231c1d219f6b Ondrej Jirman2024-04-23 631
dd231c1d219f6b Ondrej Jirman2024-04-23 632 /* we need a short
delay to allow the chip time to power on */
dd231c1d219f6b Ondrej Jirman2024-04-23 633 fsleep(1000);
dd231c1d219f6b Ondrej Jirman2024-04-23 634
be9e6229d67696 Tiberiu Breana 2015-04-27 635 ret =
stk3310_init(indio_dev);
be9e6229d67696 Tiberiu Breana 2015-04-27 636 if (ret < 0)
dd231c1d219f6b Ondrej Jirman2024-04-23 637 goto
err_
Re: [PATCH 0/3] Documentation/smatch: RST conversion and fixes
On Mon, Apr 01, 2024 at 10:45:09PM +0200, Javier Carrasco wrote: > This series converts the existing smatch.txt to RST and adds it to the > index, so it can be built together with the sparse documentation. > > When at it, a couple of small fixes has been included. > > Signed-off-by: Javier Carrasco Thanks! Applied. (Will push probably later this week). regards, dan carpenter
Re: [PATCH v2 net] ice: Fix freeing uninitialized pointers
On Sat, Mar 23, 2024 at 05:56:29PM +0100, Markus Elfring wrote: > > Automatically cleaned up pointers need to be initialized before exiting > > their scope. In this case, they need to be initialized to NULL before > > any return statement. > > * May we expect that compilers should report that affected variables > were only declared here instead of appropriately defined > (despite of attempts for scope-based resource management)? > We disabled GCC's check for uninitialized variables a long time ago because it had too many false positives. > * Did you extend detection support in the source code analysis tool “Smatch” > for a questionable implementation detail? Yes. Smatch detects this as an uninitialized variable. regards, dan carpenter
Re: [bug report] tracing/probes: Support $argN in return probe (kprobe and fprobe)
On Tue, Mar 19, 2024 at 10:10:00AM -0400, Steven Rostedt wrote:
> On Tue, 19 Mar 2024 10:19:09 +0300
> Dan Carpenter wrote:
>
> > Hello Masami Hiramatsu (Google),
> >
> > Commit 25f00e40ce79 ("tracing/probes: Support $argN in return probe
> > (kprobe and fprobe)") from Mar 4, 2024 (linux-next), leads to the
> > following Smatch static checker warning:
> >
> > kernel/trace/trace_probe.c:856 store_trace_entry_data()
> > error: uninitialized symbol 'val'.
> >
> > kernel/trace/trace_probe.c
> > 846 return;
> > 847
> > 848 for (i = 0; i < earg->size; i++) {
> > 849 struct fetch_insn *code = >code[i];
> > 850
> > 851 switch (code->op) {
> > 852 case FETCH_OP_ARG:
> > 853 val = regs_get_kernel_argument(regs,
> > code->param);
> > 854 break;
> > 855 case FETCH_OP_ST_EDATA:
> > --> 856 *(unsigned long *)((unsigned long)edata +
> > code->offset) = val;
> >
> > Probably the earg->code[i] always has FETCH_OP_ARG before
> > FETCH_OP_ST_EDATA but Smatch isn't smart enough to figure that out...
>
> Looks that way:
>
> case FETCH_OP_END:
> earg->code[i].op = FETCH_OP_ARG;
> earg->code[i].param = argnum;
> earg->code[i + 1].op = FETCH_OP_ST_EDATA;
> earg->code[i + 1].offset = offset;
> return offset;
>
> But probably should still initialize val to zero or have a WARN_ON() if
> that doesn't happen.
>
Most people use the GCC extension to initialize everything to zero so
initializing to zero really has zero cost. I always recomend people to
do it.
regards,
dan carpenter
[bug report] tracing/probes: Support $argN in return probe (kprobe and fprobe)
Hello Masami Hiramatsu (Google),
Commit 25f00e40ce79 ("tracing/probes: Support $argN in return probe
(kprobe and fprobe)") from Mar 4, 2024 (linux-next), leads to the
following Smatch static checker warning:
kernel/trace/trace_probe.c:856 store_trace_entry_data()
error: uninitialized symbol 'val'.
kernel/trace/trace_probe.c
846 return;
847
848 for (i = 0; i < earg->size; i++) {
849 struct fetch_insn *code = >code[i];
850
851 switch (code->op) {
852 case FETCH_OP_ARG:
853 val = regs_get_kernel_argument(regs,
code->param);
854 break;
855 case FETCH_OP_ST_EDATA:
--> 856 *(unsigned long *)((unsigned long)edata +
code->offset) = val;
Probably the earg->code[i] always has FETCH_OP_ARG before
FETCH_OP_ST_EDATA but Smatch isn't smart enough to figure that out...
857 break;
858 case FETCH_OP_END:
859 goto end;
860 default:
861 break;
862 }
863 }
864 end:
865 return;
866 }
regards,
dan carpenter
[PATCH v3] vduse: Fix off by one in vduse_dev_mmap()
The dev->vqs[] array has "dev->vq_num" elements. It's allocated in
vduse_dev_init_vqs(). Thus, this > comparison needs to be >= to avoid
reading one element beyond the end of the array.
Add an array_index_nospec() as well to prevent speculation issues.
Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
Signed-off-by: Dan Carpenter
---
v2: add array_index_nospec()
v3: I accidentally corrupted v2. Try again.
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index b7a1fb88c506..eb914084c650 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1532,9 +1532,10 @@ static int vduse_dev_mmap(struct file *file, struct
vm_area_struct *vma)
if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;
- if (index > dev->vq_num)
+ if (index >= dev->vq_num)
return -EINVAL;
+ index = array_index_nospec(index, dev->vq_num);
vq = dev->vqs[index];
vaddr = vq->vdpa_reconnect_vaddr;
if (vaddr == 0)
--
2.43.0
Re: [PATCH v2] vduse: Fix off by one in vduse_dev_mmap()
On Wed, Feb 28, 2024 at 12:53:28PM -0500, Stefan Hajnoczi wrote:
> On Wed, 28 Feb 2024 at 12:44, Dan Carpenter wrote:
> >
> > The dev->vqs[] array has "dev->vq_num" elements. It's allocated in
> > vduse_dev_init_vqs(). Thus, this > comparison needs to be >= to avoid
> > reading one element beyond the end of the array.
> >
> > Add an array_index_nospec() as well to prevent speculation issues.
> >
> > Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
> > Signed-off-by: Dan Carpenter
> > ---
> > v2: add array_index_nospec().
>
> Did you forget to update the patch, I don't see array_index_nospec()?
>
> >
> > drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
^
I updated the patch but the thing about vim is that every time you
press a button it does something unexpected. Vim ate my homework.
regards,
dan carpenter
[PATCH v2] vduse: Fix off by one in vduse_dev_mmap()
The dev->vqs[] array has "dev->vq_num" elements. It's allocated in
vduse_dev_init_vqs(). Thus, this > comparison needs to be >= to avoid
reading one element beyond the end of the array.
Add an array_index_nospec() as well to prevent speculation issues.
Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
Signed-off-by: Dan Carpenter
---
v2: add array_index_nospec().
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index b7a1fb88c506..eb914084c650 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1532,9 +1532,10 @@ static int vduse_dev_mmap(struct file *file, struct
vm_area_struct *vma)
if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;
- if (index > dev->vq_num)
+ if (index >= dev->vq_num)
return -EINVAL;
vq = dev->vqs[index];
vaddr = vq->vdpa_reconnect_vaddr;
if (vaddr == 0)
--
2.43.0
Re: [PATCH] vduse: Fix off by one in vduse_dev_mmap()
On Tue, Feb 27, 2024 at 10:48:49AM -0500, Michael S. Tsirkin wrote:
> On Tue, Feb 27, 2024 at 06:21:46PM +0300, Dan Carpenter wrote:
> > The dev->vqs[] array has "dev->vq_num" elements. It's allocated in
> > vduse_dev_init_vqs(). Thus, this > comparison needs to be >= to avoid
> > reading one element beyond the end of the array.
> >
> > Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
> > Signed-off-by: Dan Carpenter
>
>
> Oh wow and does this not come from userspace? If yes we
> need the speculation magic macro when using the index, do we not?
>
Yes, it does come from userspace.
To be honest, I'm not sure about speculation. The similar places in
this file protect against speculation so, probably? I'll resend the
patch.
regards,
dan carpenter
[PATCH] vduse: Fix off by one in vduse_dev_mmap()
The dev->vqs[] array has "dev->vq_num" elements. It's allocated in
vduse_dev_init_vqs(). Thus, this > comparison needs to be >= to avoid
reading one element beyond the end of the array.
Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_user/vduse_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index b7a1fb88c506..9150c8281953 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1532,7 +1532,7 @@ static int vduse_dev_mmap(struct file *file, struct
vm_area_struct *vma)
if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;
- if (index > dev->vq_num)
+ if (index >= dev->vq_num)
return -EINVAL;
vq = dev->vqs[index];
--
2.43.0
Re: [PATCH v2] acpi/ghes: Prevent sleeping with spinlock held
On Fri, Feb 16, 2024 at 05:17:20PM -0800, Dan Williams wrote: > > commit 807fbe9cac9b190dab83e3ff377a30d18859c8ab > > Author: Ira Weiny > > Date: Wed Feb 14 15:25:24 2024 -0800 > > > > Revert "acpi/ghes: Process CXL Component Events" > > > > This reverts commit 671a794c33c6e048ca5cedd5ad6af44d52d5d7e5. > > Even reverts need changelogs, I can add one. I got conflicts trying to > apply this to current fixes branch. I think I am going to just > surgically backout the drivers/acpi/apei/ghes.c changes. The `git revert` command comes from ancient times and it just sets people up for failure... :/ regards, dan carpenter
[bug report] eventfs: Read ei->entries before ei->children in eventfs_iterate()
Hello Steven Rostedt (Google),
The patch 704f960dbee2: "eventfs: Read ei->entries before
ei->children in eventfs_iterate()" from Jan 4, 2024 (linux-next),
leads to the following Smatch static checker warning:
fs/tracefs/event_inode.c:775 eventfs_iterate()
warn: missing error code here? 'create_file_dentry()' failed. 'ret' =
'0'
fs/tracefs/event_inode.c
749 /*
750 * Need to create the dentries and inodes to have a consistent
751 * inode number.
752 */
753 ret = 0;
Should this assignment be inside the loop?
754
755 /* Start at 'c' to jump over already read entries */
756 for (i = c; i < ei->nr_entries; i++, ctx->pos++) {
757 void *cdata = ei->data;
758
759 entry = >entries[i];
760 name = entry->name;
761
762 mutex_lock(_mutex);
763 /* If ei->is_freed then just bail here, nothing more to
do */
764 if (ei->is_freed) {
765 mutex_unlock(_mutex);
766 goto out;
On the second iteration through the loop then ret is an error code
767 }
768 r = entry->callback(name, , , );
769 mutex_unlock(_mutex);
770 if (r <= 0)
771 continue;
that comes from r = entry->callback(). Except, hm, none of the callback
currently return anything except zero or one so it's not an issue yet.
772
773 dentry = create_file_dentry(ei, i, ei_dentry, name,
mode, cdata, fops);
774 if (!dentry)
--> 775 goto out;
776 ino = dentry->d_inode->i_ino;
777 dput(dentry);
778
779 if (!dir_emit(ctx, name, strlen(name), ino, DT_REG))
780 goto out;
781 }
782
783 /* Subtract the skipped entries above */
784 c -= min((unsigned int)c, (unsigned int)ei->nr_entries);
785
regards,
dan carpenter
Re: [PATCH] ipvs: add a stateless type of service and a stateless Maglev hashing scheduler
Hi Lev,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Lev-Pantiukhin/ipvs-add-a-stateless-type-of-service-and-a-stateless-Maglev-hashing-scheduler/20231204-232344
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git
master
patch link:
https://lore.kernel.org/r/20231204152020.472247-1-kndrvt%40yandex-team.ru
patch subject: [PATCH] ipvs: add a stateless type of service and a stateless
Maglev hashing scheduler
config: i386-randconfig-141-20231207
(https://download.01.org/0day-ci/archive/20231207/202312070849.i9gwwsh0-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20231207/202312070849.i9gwwsh0-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202312070849.i9gwwsh0-...@intel.com/
New smatch warnings:
net/netfilter/ipvs/ip_vs_core.c:545 ip_vs_schedule() error: uninitialized
symbol 'need_state'.
vim +/need_state +545 net/netfilter/ipvs/ip_vs_core.c
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 440 struct ip_vs_conn *
190ecd27cd7294 net/netfilter/ipvs/ip_vs_core.c Julian Anastasov
2010-10-17 441 ip_vs_schedule(struct ip_vs_service *svc, struct sk_buff *skb,
d4383f04d145cc net/netfilter/ipvs/ip_vs_core.c Jesper Dangaard Brouer
2012-09-26 442 struct ip_vs_proto_data *pd, int *ignored,
d4383f04d145cc net/netfilter/ipvs/ip_vs_core.c Jesper Dangaard Brouer
2012-09-26 443 struct ip_vs_iphdr *iph)
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 444 {
9330419d9aa4f9 net/netfilter/ipvs/ip_vs_core.c Hans Schillstrom
2011-01-03 445 struct ip_vs_protocol *pp = pd->pp;
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 446 struct ip_vs_conn *cp = NULL;
ceec4c38168184 net/netfilter/ipvs/ip_vs_core.c Julian Anastasov
2013-03-22 447 struct ip_vs_scheduler *sched;
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 448 struct ip_vs_dest *dest;
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 449 __be16 _ports[2], *pptr, cport, vport;
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 450 const void *caddr, *vaddr;
3575792e005dc9 net/netfilter/ipvs/ip_vs_core.c Julian Anastasov
2010-09-17 451 unsigned int flags;
b276d504bee439 net/netfilter/ipvs/ip_vs_core.c Lev Pantiukhin
2023-12-04 452 bool need_state;
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 453
190ecd27cd7294 net/netfilter/ipvs/ip_vs_core.c Julian Anastasov
2010-10-17 454 *ignored = 1;
2f74713d1436b7 net/netfilter/ipvs/ip_vs_core.c Jesper Dangaard Brouer
2012-09-26 455 /*
2f74713d1436b7 net/netfilter/ipvs/ip_vs_core.c Jesper Dangaard Brouer
2012-09-26 456* IPv6 frags, only the first hit here.
2f74713d1436b7 net/netfilter/ipvs/ip_vs_core.c Jesper Dangaard Brouer
2012-09-26 457*/
6b3d933000cbe5 net/netfilter/ipvs/ip_vs_core.c Gao Feng
2017-11-13 458 pptr = frag_safe_skb_hp(skb, iph->len, sizeof(_ports),
_ports);
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 459 if (pptr == NULL)
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 460 return NULL;
^1da177e4c3f41 net/ipv4/ipvs/ip_vs_core.c Linus Torvalds
2005-04-16 461
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 462 if (likely(!ip_vs_iph_inverse(iph))) {
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 463 cport = pptr[0];
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 464 caddr = >saddr;
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 465 vport = pptr[1];
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 466 vaddr = >daddr;
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 467 } else {
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 468 cport = pptr[1];
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 469 caddr = >daddr;
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 470 vport = pptr[0];
ee78378f976488 net/netfilter/ipvs/ip_vs_core.c Alex Gartrell
2015-08-26 471 vaddr = >saddr;
ee78378f976488 net/netfilter/ipvs
Re: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
On Fri, Oct 27, 2023 at 03:00:00PM +0300, Dan Carpenter wrote: > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2579 #ifdef > CONFIG_DEBUG_INFO_BTF_MODULES > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2580 > /* .BTF is not SHF_ALLOC and will get removed, so sanitize pointer */ > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2581 > mod->btf_data = NULL; > 607c543f939d8c kernel/module.c Andrii Nakryiko 2020-11-20 2582 #endif > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2583 > /* > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2584 > * We want to free module_init, but be aware that kallsyms may be > 0be964be0d4508 kernel/module.c Peter Zijlstra 2015-05-27 2585 > * walking this with preempt disabled. In all the failure paths, we > cb2f55369d3a9e kernel/module.c Paul E. McKenney 2018-11-06 2586 > * call synchronize_rcu(), but we don't want to slow down the success > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2587 > * path. module_memfree() cannot be called in an interrupt, so do the > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2588 > * work and call synchronize_rcu() in a work queue. > 1a7b7d9220819a kernel/module.c Rick Edgecombe 2019-04-25 2589 > * > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2590 > * Note that module_alloc() on most architectures creates W+X page > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2591 > * mappings which won't be cleaned up until do_free_init() runs. Any > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2592 > * code such as mark_rodata_ro() which depends on those mappings to > ae646f0b9ca135 kernel/module.c Jeffrey Hugo 2018-05-11 2593 > * be cleaned up needs to sync with the queued work - ie > cb2f55369d3a9e kernel/module.c Paul E. McKenney 2018-11-06 2594 > * rcu_barrier() > c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2595 > */ > 36022a47582048 kernel/module/main.c Joey Jiao2023-10-13 2596 > if (!IS_ENABLED(CONFIG_MODULE_DISABLE_INIT_FREE) && > 36022a47582048 kernel/module/main.c Joey Jiao2023-10-13 2597 > llist_add(>node, _free_list)) > > Let's not allocate freeinit if CONFIG_MODULE_DISABLE_INIT_FREE is not > enabled. Wait. It's the other way around actually. freeinit isn't used if CONFIG_MODULE_DISABLE_INIT_FREE is enabled. regards, dan carpenter
Re: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
Hi Joey,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Joey-Jiao/module-Add-CONFIG_MODULE_DISABLE_INIT_FREE-option/20231017-115509
base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
modules-next
patch link:
https://lore.kernel.org/r/20231013062711.28852-1-quic_jiangenj%40quicinc.com
patch subject: [PATCH v5] module: Add CONFIG_MODULE_DISABLE_INIT_FREE option
config: x86_64-randconfig-161-20231026
(https://download.01.org/0day-ci/archive/20231027/202310271751.28pkvu4k-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20231027/202310271751.28pkvu4k-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202310271751.28pkvu4k-...@intel.com/
smatch warnings:
kernel/module/main.c:2608 do_init_module() warn: possible memory leak of
'freeinit'
vim +/freeinit +2608 kernel/module/main.c
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2517
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2518
freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2519 if
(!freeinit) {
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2520
ret = -ENOMEM;
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2521
goto fail;
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2522 }
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2523
freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2524
freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
ac3b4328392344 kernel/module/main.c Song Liu 2023-02-06 2525
freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2526
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2527
do_mod_ctors(mod);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2528 /*
Start the module */
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2529 if
(mod->init != NULL)
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2530
ret = do_one_initcall(mod->init);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2531 if (ret
< 0) {
c749637909eea5 kernel/module.c Rusty Russell2015-01-20 2532
goto fail_free_freeinit;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2533 }
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2534 if (ret
> 0) {
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2535
pr_warn("%s: '%s'->init suspiciously returned %d, it should "
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2536
"follow 0/-E convention\n"
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2537
"%s: loading module anyway...\n",
bddb12b32f90c5 kernel/module.c Andrew Morton2013-11-12 2538
__func__, mod->name, ret, __func__);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2539
dump_stack();
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2540 }
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2541
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2542 /* Now
it's a first class citizen! */
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2543
mod->state = MODULE_STATE_LIVE;
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2544
blocking_notifier_call_chain(_notify_list,
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2545
MODULE_STATE_LIVE, mod);
34e1169d996ab1 kernel/module.c Kees Cook2012-10-16 2546
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2547 /*
Delay uevent until module has finished its init routine */
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2548
kobject_uevent(>mkobj.kobj, KOBJ_ADD);
38dc717e97153e kernel/module.c Jessica Yu 2020-11-27 2549
774a1221e862b3 kernel/module.c Tejun Heo2013-01-15 2550 /*
774a1221e862b3 kernel/module.c Tejun Heo2013-01-15 2551* We
need to finish all async code before the module init sequence
67d6212afda218 kernel/module.c Igor Pylypiv 2022-01-27 2552* is
done. This has potential to deadlock if synchronou
[PATCH] tracing: Fix a NULL vs IS_ERR() bug in event_subsystem_dir()
The eventfs_create_dir() function returns error pointers, it never returns
NULL. Update the check to reflect that.
Fixes: 5790b1fb3d67 ("eventfs: Remove eventfs_file and just use eventfs_inode")
Signed-off-by: Dan Carpenter
---
kernel/trace/trace_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index db46d2116500..f9e3e24d8796 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2354,7 +2354,7 @@ event_subsystem_dir(struct trace_array *tr, const char
*name,
nr_entries = ARRAY_SIZE(system_entries);
ei = eventfs_create_dir(name, parent, system_entries, nr_entries, dir);
- if (!ei) {
+ if (IS_ERR(ei)) {
pr_warn("Failed to create system directory %s\n", name);
__put_system(system);
goto out_free;
--
2.42.0
[bug report] eventfs: Remove eventfs_file and just use eventfs_inode
Hello Steven Rostedt (Google),
The patch 5790b1fb3d67: "eventfs: Remove eventfs_file and just use
eventfs_inode" from Oct 4, 2023 (linux-next), leads to the following
Smatch static checker warning:
fs/tracefs/event_inode.c:782 eventfs_create_events_dir()
error: potential null dereference 'ei'. (kzalloc returns null)
fs/tracefs/event_inode.c
721 struct eventfs_inode *eventfs_create_events_dir(const char *name,
struct dentry *parent,
722 const struct
eventfs_entry *entries,
723 int size, void *data)
724 {
725 struct dentry *dentry = tracefs_start_creating(name, parent);
726 struct eventfs_inode *ei;
727 struct tracefs_inode *ti;
728 struct inode *inode;
729
730 if (security_locked_down(LOCKDOWN_TRACEFS))
731 return NULL;
I think these error paths should undo the tracefs_start_creating()
instead of returning directly.
732
733 if (IS_ERR(dentry))
734 return (struct eventfs_inode *)dentry;
Same.
735
736 ei = kzalloc(sizeof(*ei), GFP_KERNEL);
737 if (!ei)
738 goto fail;
"ei" is NULL
739
740 inode = tracefs_get_inode(dentry->d_sb);
741 if (unlikely(!inode))
742 goto fail;
743
744 if (size) {
745 ei->d_children = kzalloc(sizeof(*ei->d_children) *
size, GFP_KERNEL);
746 if (!ei->d_children)
747 goto fail;
748 }
749
750 ei->dentry = dentry;
751 ei->entries = entries;
752 ei->nr_entries = size;
753 ei->data = data;
754 ei->name = kstrdup_const(name, GFP_KERNEL);
755 if (!ei->name)
756 goto fail;
757
758 INIT_LIST_HEAD(>children);
759 INIT_LIST_HEAD(>list);
760
761 ti = get_tracefs(inode);
762 ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE;
763 ti->private = ei;
764
765 inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
766 inode->i_op = _root_dir_inode_operations;
767 inode->i_fop = _file_operations;
768
769 /* directory inodes start off with i_nlink == 2 (for "." entry)
*/
770 inc_nlink(inode);
771 d_instantiate(dentry, inode);
772 inc_nlink(dentry->d_parent->d_inode);
773 fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
774 tracefs_end_creating(dentry);
775
776 /* Will call dput when the directory is removed */
777 dget(dentry);
778
779 return ei;
780
781 fail:
--> 782 kfree(ei->d_children);
^^
Crash
783 kfree(ei);
784 tracefs_failed_creating(dentry);
785 return ERR_PTR(-ENOMEM);
786 }
regards,
dan carpenter
Re: [PATCH] tracing/eprobe: drop unneeded breaks
On Sat, Sep 30, 2023 at 06:19:02PM +0900, Masami Hiramatsu wrote:
> On Fri, 29 Sep 2023 13:37:08 +0200 (CEST)
> Julia Lawall wrote:
>
> >
> >
> > On Fri, 29 Sep 2023, Masami Hiramatsu wrote:
> >
> > > On Thu, 28 Sep 2023 12:43:34 +0200
> > > Julia Lawall wrote:
> > >
> > > > Drop break after return.
> > > >
> > >
> > > Good catch! This looks good to me.
> > >
> > > Acked-by: Masami Hiramatsu (Google)
> > >
> > > And
> > >
> > > Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> >
> > Thanks. I didn't include that because it's not a bug. But it does break
> > Coccinelle, which is how I noticed it.
>
> OK, I got it. I thought it may cause a compiler warning because the
> 'break' never be executed. (maybe it is just a flow-control word,
> so it may not need to be warned, but a bit storange.)
I don't think GCC warns about unreachable code, but yeah, in Smatch
unreachable break statements do not trigger a warning. People like
to add extra break statements to switch statements.
regards,
dan carpenter
Re: [PATCH] staging: rtl8723bs: os_dep: remove multiple return
On Sun, Apr 18, 2021 at 11:02:33PM +0530, Saurav Girepunje wrote:
> on sdio_intf.c rtw_sdio_suspend call we have multiple
> return which can replace by goto exit. As in all the places
> return value is 0.
>
> Signed-off-by: Saurav Girepunje
> ---
> drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> index a9a9631dd23c..3e566cf97f6e 100644
> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> @@ -445,14 +445,17 @@ static int rtw_sdio_suspend(struct device *dev)
> struct debug_priv *pdbgpriv = >drv_dbg;
>
> if (padapter->bDriverStopped)
> - return 0;
> + goto exit;
>
> if (pwrpriv->bInSuspend) {
> pdbgpriv->dbg_suspend_error_cnt++;
> - return 0;
> + goto exit;
> }
>
> - return rtw_suspend_common(padapter);
> + rtw_suspend_common(padapter);
Also it's weird that your previous patch made rtw_suspend_common()
return zero unconditionally. But now we're modifying the only caller to
not check the return. Just make rtw_suspend_common() void and change
this to:
rtw_suspend_common(padapter);
return 0;
regards,
dan carpenter
Re: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
On Mon, Apr 19, 2021 at 10:20:13AM +0200, Geert Uytterhoeven wrote:
> Hi Alice,
>
> CC Arnd (soc_device_match() author)
>
> On Mon, Apr 19, 2021 at 6:28 AM Alice Guo (OSS) wrote:
> > From: Alice Guo
> >
> > In i.MX8M boards, the registration of SoC device is later than caam
> > driver which needs it. Caam driver needs soc_device_match to provide
> > -EPROBE_DEFER when no SoC device is registered and no
> > early_soc_dev_attr.
>
> I'm wondering if this is really a good idea: soc_device_match() is a
> last-resort low-level check, and IMHO should be made available early on,
> so there is no need for -EPROBE_DEFER.
>
> >
> > Signed-off-by: Alice Guo
>
> Thanks for your patch!
>
> > --- a/drivers/base/soc.c
> > +++ b/drivers/base/soc.c
> > @@ -110,6 +110,7 @@ static void soc_release(struct device *dev)
> > }
> >
> > static struct soc_device_attribute *early_soc_dev_attr;
> > +static bool soc_dev_attr_init_done = false;
>
> Do you need this variable?
>
> >
> > struct soc_device *soc_device_register(struct soc_device_attribute
> > *soc_dev_attr)
> > {
> > @@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct
> > soc_device_attribute *soc_dev_attr
> > return ERR_PTR(ret);
> > }
> >
> > + soc_dev_attr_init_done = true;
> > return soc_dev;
> >
> > out3:
> > @@ -246,6 +248,9 @@ const struct soc_device_attribute *soc_device_match(
> > if (!matches)
> > return NULL;
> >
> > + if (!soc_dev_attr_init_done && !early_soc_dev_attr)
>
> if (!soc_bus_type.p && !early_soc_dev_attr)
There is one place checking this already. We could wrap it in a helper
function:
static bool device_init_done(void)
{
return soc_bus_type.p ? true : false;
}
regards,
dan carpenter
Re: [PATCH] staging: rtl8723bs: os_dep: remove multiple return
On Sun, Apr 18, 2021 at 11:02:33PM +0530, Saurav Girepunje wrote: > on sdio_intf.c rtw_sdio_suspend call we have multiple > return which can replace by goto exit. As in all the places > return value is 0. > Why? Do nothing gotos just make the code harder to read and introduce forgot to set the error code bugs. regards, dan carpenter
Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning
On Sat, Apr 17, 2021 at 09:31:32PM +, David Laight wrote: > From: Mauro Carvalho Chehab > > Sent: 17 April 2021 19:56 > > > > Em Sat, 17 Apr 2021 21:06:27 +0530 > > Ashish Kalra escreveu: > > > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > > for this file. Logical and bitwise OR are basically the same in this > > > context so it doesn't cause a runtime bug. But let's change it to > > > logical OR to make it cleaner and silence the Sparse warning. > > The old code is very likely to by slightly more efficient. > > It may not matter here, but it might in a really hot path. > > Since !x | !y and !x || !y always have the same value > why is sparse complaining at all. > Smatch doesn't warn about | vs || if both sides are true/false. But I've occasionally asked people if they were trying to do a fast path optimization but it's always just a typo. regards, dan carpenter
Re: [PATCH v2] fs/btrfs: Fix uninitialized variable
On Sat, Apr 17, 2021 at 04:36:16PM +0100, Khaled ROMDHANI wrote:
> As reported by the Coverity static analysis.
> The variable zone is not initialized which
> may causes a failed assertion.
>
> Addresses-Coverity: ("Uninitialized variables")
> Signed-off-by: Khaled ROMDHANI
> ---
> v2: add a default case as proposed by David Sterba
> ---
> fs/btrfs/zoned.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index eeb3ebe11d7a..82527308d165 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -143,6 +143,9 @@ static inline u32 sb_zone_number(int shift, int mirror)
> case 0: zone = 0; break;
> case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break;
> case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break;
It took me a while to spot these break statements.
> + default:
> + zone = 0;
> + break;
This break needs to be indented one more tab.
> }
>
> ASSERT(zone <= U32_MAX);
regards,
dan carpenter
[PATCH] mtd: rawnand: silence static checker warning in nand_setup_interface()
Smatch complains that the error code is not set on this error path:
drivers/mtd/nand/raw/nand_base.c:842 nand_setup_interface()
warn: missing error code 'ret'
But actually returning success is intentional because the NAND chip will
still work in mode 0. This patch adds a "ret = 0;" assignment to make
the intent more clear and to silence the static checker warning. It
doesn't affect the compiled code because GCC optimises the assignment
away.
Signed-off-by: Dan Carpenter
---
drivers/mtd/nand/raw/nand_base.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index fb072c95..e29e5b3d31bf 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -880,6 +880,7 @@ static int nand_setup_interface(struct nand_chip *chip, int
chipnr)
if (tmode_param[0] != chip->best_interface_config->timings.mode) {
pr_warn("timing mode %d not acknowledged by the NAND chip\n",
chip->best_interface_config->timings.mode);
+ ret = 0;
goto err_reset_chip;
}
--
2.30.2
[PATCH] platform/surface: aggregator: fix a bit test
The "funcs" variable is a u64. If "func" is more than 31 then the
BIT() shift will wrap instead of testing the high bits.
Fixes: c167b9c7e3d6 ("platform/surface: Add Surface Aggregator subsystem")
Reported-by: kernel test robot
Signed-off-by: Dan Carpenter
---
drivers/platform/surface/aggregator/controller.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/surface/aggregator/controller.c
b/drivers/platform/surface/aggregator/controller.c
index 00e38284885a..69e86cd599d3 100644
--- a/drivers/platform/surface/aggregator/controller.c
+++ b/drivers/platform/surface/aggregator/controller.c
@@ -1040,7 +1040,7 @@ static int ssam_dsm_load_u32(acpi_handle handle, u64
funcs, u64 func, u32 *ret)
union acpi_object *obj;
u64 val;
- if (!(funcs & BIT(func)))
+ if (!(funcs & BIT_ULL(func)))
return 0; /* Not supported, leave *ret at its default value */
obj = acpi_evaluate_dsm_typed(handle, _SSH_DSM_GUID,
--
2.30.2
Re: [PATCH] mtd: rawnand: fix an error code in nand_setup_interface()
On Fri, Apr 16, 2021 at 05:00:40PM +0200, Miquel Raynal wrote:
> Hi Dan,
>
> Dan Carpenter wrote on Wed, 14 Apr 2021
> 08:56:33 +0300:
>
> > We should return an error code if the timing mode is not acknowledged
> > by the NAND chip.
>
> This truly is questionable (and I am not yet decided whether the answer
> should be yes or no).
>
> Returning an error here would produce the entire boot sequence to fail,
> even though the NAND chip would work in mode 0.
>
> Not returning an error would print the below warning (so the
> user/developer is warned) and continue the boot with the slowest
> timing interface.
>
> Honestly I would be more in favor of letting things as they are
> because I don't think this may be considered as a buggy situation, but I
> am open to discussion.
>
If we decided that the original code is correct then one way to silence
the warning would be to do:
if (tmode_param[0] != chip->best_interface_config->timings.mode) {
pr_warn("timing mode %d not acknowledged by the NAND chip\n",
chip->best_interface_config->timings.mode);
ret = 0;
goto err_reset_chip;
}
Setting "ret = 0;" right before the goto makes the code look more
intentional to human readers as well.
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line
On Fri, Apr 16, 2021 at 11:37:28AM +0300, Sakari Ailus wrote: > Hi Dan, > > On Fri, Apr 16, 2021 at 08:49:41AM +0300, Dan Carpenter wrote: > > On Fri, Apr 16, 2021 at 12:21:58AM +0300, Sakari Ailus wrote: > > > On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > > > > On Thu, Apr 15, 2021 at 08:57:04PM +0100, Matthew Wilcox wrote: > > > > > On Thu, Apr 15, 2021 at 10:49:55PM +0300, Sakari Ailus wrote: > > > > > > On Thu, Apr 15, 2021 at 06:14:09PM +0100, Matthew Wilcox wrote: > > > > > > > On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro > > > > > > > wrote: > > > > > > > > -const struct atomisp_format_bridge > > > > > > > > *get_atomisp_format_bridge_from_mbus( > > > > > > > > -u32 mbus_code); > > > > > > > > +const struct atomisp_format_bridge* > > > > > > > > +get_atomisp_format_bridge_from_mbus(u32 mbus_code); > > > > > > > > > > > > > > No, this does not match coding style. Probably best to break the > > > > > > > 80-column guideline in this instance. Best would be to have a > > > > > > > function > > > > > > > > > > > > Having the return type on the previous line is perfectly fine. > > > > > > There should > > > > > > be a space before the asterisk though. > > > > > > > > > > No, it's not. Linus has ranted about that before. > > > > > > > > Found it. > > > > https://lore.kernel.org/lkml/1054519757.161...@palladium.transmeta.com/ > > > > > > Two decades ago, really? > > > > > > This is simply one of the practical means how you split long function > > > declarations and avoid overly long lines. Not my favourite though, but > > > still better than those long lines. > > > > I've always thought we allow either style, but it has to be done > > consistently within the file. I was pretty sure that was policy but > > it's another thing that goes back decades so I don't have a reference. > > It shouldn't be about breaking up long lines. > > > > > > > > My personal preference would be to wrap at the opening parenthesis and > > > indent by just a tab, but I know many people who disagree with that... > > > > If you're running into the 80 character limit, then it's fine to use > > two tabs. I think we have been rejecting patches that push align the > > parameters but push past the 80 character limit. Using one tab is > > confusing because it makes the decalarations line up with the code. > > Interesting. Do you have an example of this? I've thought checkpatch.pl > gave a warning if the line ended with an opening parenthesis no matter > what. The prefered style is still aligning with the parentheses but if you have to choose between a warning about going over the limit or a warning about aligning then probably it's fine to not align. I can't find an example, but I'm pretty sure we've been rejecting patches that align the parameters but now go over the 80/100 char limit. regards, dan carpenter
Re: [Outreachy kernel] [PATCH v2] staging: media: atomisp: pci: Change line break to avoid an open parenthesis at the end of the line
On Fri, Apr 16, 2021 at 12:21:58AM +0300, Sakari Ailus wrote: > On Thu, Apr 15, 2021 at 08:59:41PM +0100, Matthew Wilcox wrote: > > On Thu, Apr 15, 2021 at 08:57:04PM +0100, Matthew Wilcox wrote: > > > On Thu, Apr 15, 2021 at 10:49:55PM +0300, Sakari Ailus wrote: > > > > On Thu, Apr 15, 2021 at 06:14:09PM +0100, Matthew Wilcox wrote: > > > > > On Thu, Apr 15, 2021 at 02:08:19PM -0300, Aline Santana Cordeiro > > > > > wrote: > > > > > > -const struct atomisp_format_bridge > > > > > > *get_atomisp_format_bridge_from_mbus( > > > > > > -u32 mbus_code); > > > > > > +const struct atomisp_format_bridge* > > > > > > +get_atomisp_format_bridge_from_mbus(u32 mbus_code); > > > > > > > > > > No, this does not match coding style. Probably best to break the > > > > > 80-column guideline in this instance. Best would be to have a > > > > > function > > > > > > > > Having the return type on the previous line is perfectly fine. There > > > > should > > > > be a space before the asterisk though. > > > > > > No, it's not. Linus has ranted about that before. > > > > Found it. > > https://lore.kernel.org/lkml/1054519757.161...@palladium.transmeta.com/ > > Two decades ago, really? > > This is simply one of the practical means how you split long function > declarations and avoid overly long lines. Not my favourite though, but > still better than those long lines. I've always thought we allow either style, but it has to be done consistently within the file. I was pretty sure that was policy but it's another thing that goes back decades so I don't have a reference. It shouldn't be about breaking up long lines. > > My personal preference would be to wrap at the opening parenthesis and > indent by just a tab, but I know many people who disagree with that... If you're running into the 80 character limit, then it's fine to use two tabs. I think we have been rejecting patches that push align the parameters but push past the 80 character limit. Using one tab is confusing because it makes the decalarations line up with the code. regards, dan carpenter
Re: [kbuild] drivers/gpu/drm/msm/adreno/a3xx_gpu.c:600 a3xx_gpu_init() error: passing non negative 1 to ERR_PTR
On Thu, Apr 15, 2021 at 04:21:01PM -0700, Rob Clark wrote:
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 571 icc_path =
> > > devm_of_icc_get(>dev, "gfx-mem");
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 572 ret =
> > > IS_ERR(icc_path);
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 573 if (ret)
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 574 goto fail;
> > >
> > > IS_ERR() returns/true false so this will lead to an Oops in the caller.
> > >
> > > icc_path = devm_of_icc_get(>dev, "gfx-mem");
> > > if (IS_ERR(icc_path)) {
> > > ret = PTR_ERR(icc_path);
> > > goto fail;
> > > }
> > Agree.
> >
> > >
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 575
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 576 ocmem_icc_path =
> > > devm_of_icc_get(>dev, "ocmem");
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 577 ret =
> > > IS_ERR(ocmem_icc_path);
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 578 if (ret) {
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 579 /* allow
> > > -ENODATA, ocmem icc is optional */
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 580 if (ret !=
> > > -ENODATA)
> > > 5785dd7a8ef0de Akhil P Oommen 2020-10-28 581
> > > goto fail;
> > >
> > > Same. ret is true/false so it can't be equal to -ENODATA, plus the
> > > caller will Oops.
> > >
> > > Btw, this patch removed the assignments:
> > >
> > > gpu->icc_path = of_icc_get(dev, "gfx-mem");
> > > gpu->ocmem_icc_path = of_icc_get(dev, "ocmem");
> > >
> > > So I think "gpu->icc_path" and "gpu->ocmem_icc_path" are always
> > > NULL/unused and they should be removed.
> > >
> > Agree. Will share a fix.
> > Thanks, Dan.
>
> gpu->ocmem_icc_path/icc_path is used on older devices.. it sounds like
> we broke some older devices and no one has noticed yet?
This is error paths and dead code. Probably no one is affected in
real life.
regards,
dan carpenter
Re: [PATCH] clk: uniphier: Fix potential infinite loop
On Fri, Apr 09, 2021 at 03:46:47PM +0900, Masahiro Yamada wrote:
> On Thu, Apr 8, 2021 at 12:25 AM Colin King wrote:
> >
> > From: Colin Ian King
> >
> > The for-loop iterates with a u8 loop counter i and compares this
> > with the loop upper limit of num_parents that is an int type.
> > There is a potential infinite loop if num_parents is larger than
> > the u8 loop counter. Fix this by making the loop counter the same
> > type as num_parents.
> >
> > Addresses-Coverity: ("Infinite loop")
> > Fixes: 734d82f4a678 ("clk: uniphier: add core support code for UniPhier
> > clock driver")
> > Signed-off-by: Colin Ian King
> > ---
> > drivers/clk/uniphier/clk-uniphier-mux.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/uniphier/clk-uniphier-mux.c
> > b/drivers/clk/uniphier/clk-uniphier-mux.c
> > index 462c84321b2d..ce219e0d2a85 100644
> > --- a/drivers/clk/uniphier/clk-uniphier-mux.c
> > +++ b/drivers/clk/uniphier/clk-uniphier-mux.c
> > @@ -34,7 +34,7 @@ static u8 uniphier_clk_mux_get_parent(struct clk_hw *hw)
> > int num_parents = clk_hw_get_num_parents(hw);
> > int ret;
> > unsigned int val;
> > - u8 i;
> > + int i;
> >
> > ret = regmap_read(mux->regmap, mux->reg, );
> > if (ret)
> > --
> > 2.30.2
> >
>
> clk_hw_get_num_parents() returns 'unsigned int', so
> I think 'num_parents' should also have been 'unsigned int'.
>
> Maybe, the loop counter 'i' also should be 'unsigned int' then?
The clk_hw_get_num_parents() function returns 0-255 so the original code
works fine.
It should basically always be "int i;" That's the safest assumption.
There are other case where it has to be size_t but in those cases I
think people should call the list iterator something else instead of "i"
like "size_t pg_idx;".
Making everthing u32 causes more bugs than it prevents. Signedness bugs
with comparing to zero, type promotion bugs, or subtraction bugs where
subtracting wraps to a high value. It's rare to loop more than INT_MAX
times in the kernel. When we do need to count about 2 million then
we're probably not going to stop counting at 4 million, we're going to
go to 10 million or higher so size_t is more appropriate than u32.
Btw, if you have a loop that does:
for (i = 0; i < UINT_MAX; i++) {
that loop works exactly the same if "i" is an int or if it's a u32
because of type promotion. So you have to look really hard to find a
place where changing a loop iterator from int to u32 fixes bug in real
life.
regards,
dan carpenter
Re: [PATCH][next] net: stmmac: replace redundant comparison with true
On Thu, Apr 15, 2021 at 09:37:57AM +0100, Colin King wrote:
> From: Colin Ian King
>
> The comparison of the u32 variable queue with <= zero is always true
> since an unsigned can never be negative. Replace the conditional
> check with the boolean true to simplify the code. The while loop
> will terminate because of the zero check on queue before queue is
> decremented.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Signed-off-by: Colin Ian King
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e3e22200a4fd..6e5b4c4b375c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1673,7 +1673,7 @@ static void stmmac_reinit_rx_buffers(struct stmmac_priv
> *priv)
> return;
>
> err_reinit_rx_buffers:
> - while (queue >= 0) {
This is an off by one from what the original developer was intending
because we're freeing the most recent queue that wasn't allocated.
In other words, we're freeing everything that we need to plus *one
more thing that we don't need to*. But it's harmless in this case:
The better fix would be to make queue an int type and do:
while (--queue >= 0)
dma_free_rx_skbufs(priv, queue);
[kbuild] Re: [PATCH 13/15] usb: dwc2: Add exit hibernation mode before removing drive
Hi Artur,
https://git-scm.com/docs/git-format-patch ]
url:
https://github.com/0day-ci/linux/commits/Artur-Petrosyan/usb-dwc2-Update-exit-hibernation-when-port-reset-is-asserted/20210415-144021
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
config: i386-randconfig-m021-20210415 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/usb/dwc2/platform.c:324 dwc2_driver_remove() error: potentially
dereferencing uninitialized 'gr'.
vim +/gr +324 drivers/usb/dwc2/platform.c
5b9974b13e3648 drivers/staging/dwc2/platform.c Matthijs Kooijman 2013-04-22
316 static int dwc2_driver_remove(struct platform_device *dev)
5b9974b13e3648 drivers/staging/dwc2/platform.c Matthijs Kooijman 2013-04-22
317 {
5b9974b13e3648 drivers/staging/dwc2/platform.c Matthijs Kooijman 2013-04-22
318 struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
319 struct dwc2_gregs_backup *gr;
^^
b46b1ef7b0da5c drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-08
320 int ret = 0;
b46b1ef7b0da5c drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-08
321
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
322 /* Exit Hibernation when driver is removed. */
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
323 if (hsotg->hibernated) {
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
@324 if (gr->gotgctl & GOTGCTL_CURMODE_HOST) {
^^^
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
325 ret = dwc2_exit_hibernation(hsotg, 0, 0, 1);
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
326 if (ret)
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
327 dev_err(hsotg->dev,
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
328 "exit hibernation failed.\n");
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
329 } else {
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
330 ret = dwc2_exit_hibernation(hsotg, 0, 0, 0);
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
331 if (ret)
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
332 dev_err(hsotg->dev,
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
333 "exit hibernation failed.\n");
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
334 }
8e4dbc0200040a drivers/usb/dwc2/platform.c Artur Petrosyan2021-04-15
335 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
kbuild mailing list -- kbu...@lists.01.org
To unsubscribe send an email to kbuild-le...@lists.01.org
Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap
I screwed up my last email and dropped Lee and Arnd from the To: headers.
Resending.
On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > ---
> > drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > index c95ae4d6a3b6b..cc14f00947781 100644
> > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > /* parsing WPA/WPA2 IE */
> > {
> > u8 *buf;
> > - u8 wpa_ie[255], rsn_ie[255];
> > + u8 *wpa_ie, *rsn_ie;
> > u16 wpa_len = 0, rsn_len = 0;
> > u8 *p;
> >
> > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > if (!buf)
> > return start;
Arnd added this return. I think it should be -ENOMEM, though?
> >
> > + wpa_ie = kzalloc(255, GFP_ATOMIC);
> > + if (!wpa_ie)
> > + return start;
>
> kfree(buf);
>
> > +
> > + rsn_ie = kzalloc(255, GFP_ATOMIC);
> > + if (!rsn_ie)
> > + return start;
>
regards,
dan carpenter
Re: [PATCH v2] staging: media: atomisp: pci: Format comments according to coding-style in file atomisp_cmd.c
On Wed, Apr 14, 2021 at 05:42:44PM -0300, Aline Santana Cordeiro wrote:
> @@ -90,18 +92,14 @@ struct camera_mipi_info
> *atomisp_to_sensor_mipi_info(struct v4l2_subdev *sd)
> return (struct camera_mipi_info *)v4l2_get_subdev_hostdata(sd);
> }
>
> -/*
> - * get struct atomisp_video_pipe from v4l2 video_device
> - */
> +/* get struct atomisp_video_pipe from v4l2 video_device */
This code is obvious and the comment doesn't add anything except noise.
Just delete it. Same for a lot of the other one line comments
describing functions in this patch.
> struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev)
> {
> return (struct atomisp_video_pipe *)
> container_of(dev, struct atomisp_video_pipe, vdev);
> }
>
> -/*
> - * get struct atomisp_acc_pipe from v4l2 video_device
> - */
> +/* get struct atomisp_acc_pipe from v4l2 video_device */
> struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct video_device *dev)
> {
> return (struct atomisp_acc_pipe *)
> @@ -269,7 +267,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
> ATOMISP_RUN_MODE_CONTINUOUS_CAPTURE;
> }
>
> - /* search for the target frequency by looping freq rules*/
> + /* search for the target frequency by looping freq rules */
> for (i = 0; i < dfs->dfs_table_size; i++) {
> if (curr_rules.width != dfs->dfs_table[i].width &&
> dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY)
> @@ -307,9 +305,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
> return ret;
> }
>
> -/*
> - * reset and restore ISP
> - */
> +/* reset and restore ISP */
Obvious
> int atomisp_reset(struct atomisp_device *isp)
> {
> /* Reset ISP by power-cycling it */
> @@ -338,9 +334,7 @@ int atomisp_reset(struct atomisp_device *isp)
> return ret;
> }
>
> -/*
> - * interrupt disable functions
> - */
> +/* interrupt disable functions */
Obvious
> static void disable_isp_irq(enum hrt_isp_css_irq irq)
> {
> irq_disable_channel(IRQ0_ID, irq);
> @@ -351,9 +345,7 @@ static void disable_isp_irq(enum hrt_isp_css_irq irq)
> cnd_sp_irq_enable(SP0_ID, false);
> }
>
> -/*
> - * interrupt clean function
> - */
> +/* interrupt clean function */
Obvious
> static void clear_isp_irq(enum hrt_isp_css_irq irq)
> {
> irq_clear_all(IRQ0_ID);
[ snip ]
> @@ -1918,10 +1914,7 @@ irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr)
> return IRQ_HANDLED;
> }
>
> -/*
> - * utils for buffer allocation/free
> - */
> -
> +/* utils for buffer allocation/free */
What? This one seems actively wrong.
> int atomisp_get_frame_pgnr(struct atomisp_device *isp,
> const struct ia_css_frame *frame, u32 *p_pgnr)
> {
etc.
regards,
dan carpenter
Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap
On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > ---
> > drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > index c95ae4d6a3b6b..cc14f00947781 100644
> > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > /* parsing WPA/WPA2 IE */
> > {
> > u8 *buf;
> > - u8 wpa_ie[255], rsn_ie[255];
> > + u8 *wpa_ie, *rsn_ie;
> > u16 wpa_len = 0, rsn_len = 0;
> > u8 *p;
> >
> > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > if (!buf)
> > return start;
Arnd, added this return... I don't understand why we aren't returning
-ENOMEM here.
> >
> > + wpa_ie = kzalloc(255, GFP_ATOMIC);
> > + if (!wpa_ie)
> > + return start;
>
> kfree(buf);
>
> > +
> > + rsn_ie = kzalloc(255, GFP_ATOMIC);
> > + if (!rsn_ie)
> > + return start;
>
regards,
dan carpenter
Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap
On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> ---
> drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index c95ae4d6a3b6b..cc14f00947781 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> /* parsing WPA/WPA2 IE */
> {
> u8 *buf;
> - u8 wpa_ie[255], rsn_ie[255];
> + u8 *wpa_ie, *rsn_ie;
> u16 wpa_len = 0, rsn_len = 0;
> u8 *p;
>
> @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> if (!buf)
> return start;
>
> + wpa_ie = kzalloc(255, GFP_ATOMIC);
> + if (!wpa_ie)
> + return start;
kfree(buf);
> +
> + rsn_ie = kzalloc(255, GFP_ATOMIC);
> + if (!rsn_ie)
> + return start;
kfree(buf);
kfree(wpa_ie);
> +
> rtw_get_sec_ie(pnetwork->network.ies,
> pnetwork->network.ie_length, rsn_ie, _len, wpa_ie, _len);
> RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
> ("rtw_wx_get_scan: ssid =%s\n", pnetwork->network.ssid.ssid));
> RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
> ("rtw_wx_get_scan: wpa_len =%d rsn_len =%d\n", wpa_len, rsn_len));
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH v2] staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 07:00:41PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 14, 2021 at 06:26:14PM +0200, Fabio M. De Francesco wrote:
> > Removed useless led_blink_hdl() prototype and definition. In wlancmds[]
> > the slot #60 is now set to NULL using the macro GEN_MLME_EXT_HANDLER. This
> > change has not unwanted side effects because the code in rtw_cmd.c checks
> > if the function pointer is valid before using it.
> >
> > Reported-by: Julia Lawall
> > Suggested-by: Dan Carpenter
> > Signed-off-by: Fabio M. De Francesco
> > ---
> >
> > Changes since v1: Corrected a bad solution to this issue that made use of
> > an unnecessary dummy function.
> >
> > drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +-
> > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c| 9 -
> > drivers/staging/rtl8723bs/include/rtw_mlme_ext.h | 1 -
> > 3 files changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > index 0297fbad7bce..f82dbd4f4c3d 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > @@ -150,7 +150,7 @@ static struct cmd_hdl wlancmds[] = {
> >
> > GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl) /*58*/
> > GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param),
> > set_chplan_hdl) /*59*/
> > - GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param), led_blink_hdl)
> > /*60*/
> > + GEN_MLME_EXT_HANDLER(0, NULL) /*60*/
>
> Better, but you really do not need to keep this here, right? Remove the
> "led blink command" entirely, you didn't do that here.
No, this is right. We have to put a NULL function pointer in the array
or the indexing will be off. But Fabio is correct that the struct
type should be removed.
regards,
dan carpenter
Re: [Outreachy kernel] Re: [PATCH 1/2] staging: media: atomisp: pci: Correct identation in block of conditional statements in file atomisp_v4l2.c
On Wed, Apr 14, 2021 at 04:39:20PM +0200, Julia Lawall wrote:
>
>
> On Wed, 14 Apr 2021, Dan Carpenter wrote:
>
> > On Wed, Apr 14, 2021 at 11:06:02AM -0300, Aline Santana Cordeiro wrote:
> > > Correct identation in block of conditional statements.
> > > The function "v4l2_device_unregister_subdev()" depends on
> > > the results of the macro function "list_for_each_entry_safe()".
> > >
> > > Signed-off-by: Aline Santana Cordeiro
> > > ---
> > > drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > index 0295e2e..6d853f4 100644
> > > --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > @@ -1178,7 +1178,7 @@ static void atomisp_unregister_entities(struct
> > > atomisp_device *isp)
> > > atomisp_mipi_csi2_unregister_entities(>csi2_port[i]);
> > >
> > > list_for_each_entry_safe(sd, next, >v4l2_dev.subdevs, list)
> > > - v4l2_device_unregister_subdev(sd);
> > > + v4l2_device_unregister_subdev(sd);
> > >
> >
> > It's really more common to use curly braces for list_for_each() one
> > liners.
> >
> > list_for_each_entry_safe(sd, next, >v4l2_dev.subdevs, list) {
> > v4l2_device_unregister_subdev(sd);
> > }
>
> A quick test with grep shows 4000 lines containing list_for_each that
> contain no {, out of 15000 lines containing list_for_each in all.
>
Huh... You're right. Never mind then.
regards,
dan carpenter
Re: [PATCH 1/2] staging: media: atomisp: pci: Correct identation in block of conditional statements in file atomisp_v4l2.c
On Wed, Apr 14, 2021 at 11:06:02AM -0300, Aline Santana Cordeiro wrote:
> Correct identation in block of conditional statements.
> The function "v4l2_device_unregister_subdev()" depends on
> the results of the macro function "list_for_each_entry_safe()".
>
> Signed-off-by: Aline Santana Cordeiro
> ---
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 0295e2e..6d853f4 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -1178,7 +1178,7 @@ static void atomisp_unregister_entities(struct
> atomisp_device *isp)
> atomisp_mipi_csi2_unregister_entities(>csi2_port[i]);
>
> list_for_each_entry_safe(sd, next, >v4l2_dev.subdevs, list)
> - v4l2_device_unregister_subdev(sd);
> + v4l2_device_unregister_subdev(sd);
>
It's really more common to use curly braces for list_for_each() one
liners.
list_for_each_entry_safe(sd, next, >v4l2_dev.subdevs, list) {
v4l2_device_unregister_subdev(sd);
}
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 01:52:43PM +0200, Fabio M. De Francesco wrote:
> Removed the led_blink_hdl() function (declaration and definition).
> Declared dummy_function() in include/rtw_mlme_ext.h and defined it in
> core/rtw_cmd.c. Changed the second parameter of GEN_MLME_EXT_HANDLER
> macro to make use of dummy_function().
>
> Reported-by: Julia Lawall
> Suggested-by: Dan Carpenter
> Signed-off-by: Fabio M. De Francesco
> ---
> drivers/staging/rtl8723bs/core/rtw_cmd.c | 4 +++-
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c| 9 -
> drivers/staging/rtl8723bs/include/rtw_mlme_ext.h | 3 ++-
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> index 0297fbad7bce..7b6102a2bb2c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> @@ -87,6 +87,8 @@ static struct _cmd_callback rtw_cmd_callback[] = {
> {GEN_CMD_CODE(_RunInThreadCMD), NULL},/*64*/
> };
>
> +u8 dummy_functioni(struct adapter *var0, u8 *var1) { return 0; }
> +
> static struct cmd_hdl wlancmds[] = {
> GEN_DRV_CMD_HANDLER(0, NULL) /*0*/
> GEN_DRV_CMD_HANDLER(0, NULL)
> @@ -150,7 +152,7 @@ static struct cmd_hdl wlancmds[] = {
>
> GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl) /*58*/
> GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param),
> set_chplan_hdl) /*59*/
> - GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param), led_blink_hdl)
> /*60*/
> + GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param), dummy_function)
> /*60*/
No, no. Don't create a dummy function. Do it like so:
GEN_DRV_CMD_HANDLER(0, NULL) /* 60 */
regards,
dan carpenter
Re: [PATCH] objtool: prevent memory leak in error paths
On Wed, Apr 14, 2021 at 01:45:11AM +0500, Muhammad Usama Anjum wrote:
> Memory allocated by sym and sym->name isn't being freed if some error
> occurs in elf_create_undef_symbol(). Free the sym and sym->name if error
> is detected before returning NULL.
>
> Addresses-Coverity: ("Prevent memory leak")
> Fixes: 2f2f7e47f052 ("objtool: Add elf_create_undef_symbol()")
> Signed-off-by: Muhammad Usama Anjum
> ---
> Only build has been tested.
>
Just ignore leaks from the tools/ directory. These things run and then
exit and all the memory is freed. #OldSchoolGarbageCollector
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 09:59:36AM +0200, Fabio M. De Francesco wrote: > Fine. I'm about to submit a patch. > > Is there any formal means to acknowledge (in the patch) your contribution > to the resolution of this problem? It doesn't matter. Suggested-by. regards, dan carpenter
Re: [PATCH v2 4/5] staging: rtl8192e: rectified spelling mistake and replace memcmp with ether_oui_equal
On Wed, Apr 14, 2021 at 12:26:01PM +0530, Mitali Borkar wrote:
> Added a generic function of static inline bool in
> include/linux/etherdevice.h to replace memcmp with
> ether_oui_equal throughout the execution.
> Corrected the misspelled words in this file.
>
> Signed-off-by: Mitali Borkar
> ---
>
> Changes from v1:- Rectified spelling mistake and replaced memcmp with
> ether_oui_equal.
>
> drivers/staging/rtl8192e/rtl819x_HTProc.c | 48 +++
> include/linux/etherdevice.h | 5 +++
^^^
This is networking code and not staging code, but the netdev mailing
list isn't CC'd.
> 2 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> index ec6b46166e84..ce58feb2af9a 100644
> --- a/drivers/staging/rtl8192e/rtl819x_HTProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_HTProc.c
> @@ -43,7 +43,7 @@ u16 MCS_DATA_RATE[2][2][77] = {
>810, 720, 810, 900, 900, 990} }
> };
>
> -static u8 UNKNOWN_BORADCOM[3] = {0x00, 0x14, 0xbf};
> +static u8 UNKNOWN_BROADCOM[3] = {0x00, 0x14, 0xbf};
Please pull this spelling fix into its own patch.
[ snip ]
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 2e5debc0373c..6a1a63168319 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -87,6 +87,11 @@ static inline bool is_link_local_ether_addr(const u8 *addr)
> #endif
> }
>
> +static inline bool ether_oui_equal(const u8 *addr, const u8 *oui)
> +{
> +return addr[0] == oui[0] && addr[1] == oui[1] && addr[2] == oui[2];
> +}
The indenting is messed up on this.
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 09:40:36AM +0200, Fabio Aiuto wrote:
> On Wed, Apr 14, 2021 at 08:21:50AM +0300, Dan Carpenter wrote:
> > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote:
> > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote:
> > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco wrote:
> > > > > 1) The driver doesn't call that function from anywhere else than the
> > > > > macro. 2) You have explained that the macro add its symbol to a slot
> > > > > in an array that would shift all the subsequent elements down if that
> > > > > macro is not used exactly in the line where it is.
> > > > > 3) Dan Carpenter said that that array is full of null functions (or
> > > > > empty slots?).
> > > > >
> > > > > Unless that function is called anonymously dereferencing its address
> > > > > from the position it occupies in the array, I'm not able to see what
> > > > > else means can any caller use.
> > > > >
> > > > > I know I have much less experience than you with C: what can go wrong?
> > > >
> > > > Here's where the driver calls that function:
> > > >
> > > > $ git grep wlancmds drivers/staging/rtl8723bs/
> > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl
> > > > wlancmds[]
> > > > = { drivers/staging/rtl8723bs/core/rtw_cmd.c: if
> > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) {
> > > > drivers/staging/rtl8723bs/core/rtw_cmd.c: cmd_hdl
> > > > = wlancmds[pcmd->cmdcode].h2cfuns;
> > > >
> > > OK, I had imagined an anonymous call from its location in the array (as I
> > > wrote in the last phrase of my message). However, I thought that it could
> > > have been an improbable possibility, not a real one.
> > >
> > > Linux uses a lot of interesting ideas that newcomers like me should
> > > learn.
> > > Things here are trickier than they appear at first sight.
> >
> > One trick would be to build the Smatch cross function database.
> >
> > https://www.spinics.net/lists/smatch/msg00568.html
> >
> > Then you could do:
> >
> > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl
> > file | caller | function | type | parameter | key | value |
> > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
> > uchar(*)(struct adapter*, uchar*)
> > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
> > uchar(*)(struct adapter*, uchar*)
> > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf |
> > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960
> >
> >
> > Which says that led_blink_hdl() is called as a function pointer called
> > "cmd_hdl" from rtw_cmd_thread().
> >
> > Hm... It says it can be called from either rtw_cmd_thread() function
> > (the rtl8723bs or rtl8188eu version) which is not ideal. But also
> > basically harmless so whatever...
> >
> > regards,
> > dan carpenter
> >
>
> very powerful tool.
>
> I tried this:
>
> fabio@agape:~/src/git/kernels/staging$
> ~/src/git/smatch/smatch_data/db/smdb.py led_blink_hdl
> Traceback (most recent call last):
> File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 725, in
>
> print_caller_info("", func)
> File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 366, in
> print_caller_info
> ptrs = get_function_pointers(func)
> File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 53, in
> get_function_pointers
> get_function_pointers_helper(func)
> File "/home/fabio/src/git/smatch/smatch_data/db/smdb.py", line 38, in
> get_function_pointers_helper
> cur.execute("select distinct ptr from function_ptr where function =
> '%s';" %(func))
> sqlite3.OperationalError: no such table: function_ptr
>
> I run smatch version 1.71 on Debian Buster machine
>
It takes a few hours to build the DB. The instructions are in the
email.
~/path/to/smatch/smatch_scripts/build_kernel_data.sh
regards,
dan carpenter
[PATCH] usb: typec: silence a static checker warning
Smatch complains about a potential missing error code:
drivers/usb/typec/port-mapper.c:168 typec_link_port()
warn: missing error code 'ret'
This is a false positive and returning zero is intentional. Let's
re-arrange the code to silence the warning and make the intent more
clear.
Signed-off-by: Dan Carpenter
---
drivers/usb/typec/port-mapper.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c
index fae736eb0601..9b0991bdf391 100644
--- a/drivers/usb/typec/port-mapper.c
+++ b/drivers/usb/typec/port-mapper.c
@@ -157,15 +157,17 @@ int typec_link_port(struct device *port)
{
struct device *connector;
struct port_node *node;
- int ret = 0;
+ int ret;
node = create_port_node(port);
if (IS_ERR(node))
return PTR_ERR(node);
connector = find_connector(node);
- if (!connector)
+ if (!connector) {
+ ret = 0;
goto remove_node;
+ }
ret = link_port(to_typec_port(connector), node);
if (ret)
--
2.30.2
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Wed, Apr 14, 2021 at 08:33:48AM +0200, Fabio M. De Francesco wrote:
> On Wednesday, April 14, 2021 7:21:50 AM CEST Dan Carpenter wrote:
> > On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote:
> > > On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote:
> > > > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco
> wrote:
> > > > > 1) The driver doesn't call that function from anywhere else than
> > > > > the
> > > > > macro. 2) You have explained that the macro add its symbol to a
> > > > > slot
> > > > > in an array that would shift all the subsequent elements down if
> > > > > that
> > > > > macro is not used exactly in the line where it is.
> > > > > 3) Dan Carpenter said that that array is full of null functions (or
> > > > > empty slots?).
> > > > >
> > > > > Unless that function is called anonymously dereferencing its
> > > > > address
> > > > > from the position it occupies in the array, I'm not able to see
> > > > > what
> > > > > else means can any caller use.
> > > > >
> > > > > I know I have much less experience than you with C: what can go
> > > > > wrong?
> > > >
> > > > Here's where the driver calls that function:
> > > >
> > > > $ git grep wlancmds drivers/staging/rtl8723bs/
> > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl
> > > > wlancmds[] = { drivers/staging/rtl8723bs/core/rtw_cmd.c:
> > > > if
> > > > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) {
> > > > drivers/staging/rtl8723bs/core/rtw_cmd.c:
> > > > cmd_hdl
> > > > = wlancmds[pcmd->cmdcode].h2cfuns;
> > >
> > > OK, I had imagined an anonymous call from its location in the array (as
> > > I wrote in the last phrase of my message). However, I thought that it
> > > could have been an improbable possibility, not a real one.
> > >
> > > Linux uses a lot of interesting ideas that newcomers like me should
> > > learn. Things here are trickier than they appear at first sight.
> >
> > One trick would be to build the Smatch cross function database.
> >
> > https://www.spinics.net/lists/smatch/msg00568.html
> >
> > Then you could do:
> >
> > $ ~/path/to/smatch_data/db/smdb.py led_blink_hdl
> > file | caller | function | type | parameter | key | value |
> > drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
> > uchar(*)(struct adapter*, uchar*)
> > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
> > uchar(*)(struct adapter*, uchar*)
> > drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
> > rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf |
> > 1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960
> >
> >
> > Which says that led_blink_hdl() is called as a function pointer called
> > "cmd_hdl" from rtw_cmd_thread().
> >
> > Hm... It says it can be called from either rtw_cmd_thread() function
> > (the rtl8723bs or rtl8188eu version) which is not ideal. But also
> > basically harmless so whatever...
> >
> > regards,
> > dan carpenter
> >
> Nice tool, thanks. I'll surely use it when it is needed to find out which
> callers use a function pointer.
>
> However I cannot see how it can help in this context. That function *does*
> something, even if I cannot understand why someone needs a function to test
> the initialization of a pointer. Furthermore it is actually called by
> rtw_cmd_thread() (as you found out by using smatch) that expect one of the
> two possible values that led_blink_hdl() returns.
>
> That said, what trick could I use for the purpose of getting rid of that
> function? At this point I'm not sure it could be made.
If you look at how this is called:
drivers/staging/rtl8723bs/core/rtw_cmd.c
449 memcpy(pcmdbuf, pcmd->parmbuf, pcmd->cmdsz);
450
451 if (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) {
452 cmd_hdl = wlancmds[pcmd->cmdcode].h2cfuns;
453
454 if (cmd_hdl) {
455 ret = cmd_hdl(pcmd->padapter, pcmdbuf);
[PATCH] mtd: rawnand: fix an error code in nand_setup_interface()
We should return an error code if the timing mode is not acknowledged
by the NAND chip.
Fixes: 415ae78ffb5d ("mtd: rawnand: check ONFI timings have been acked by the
chip")
Signed-off-by: Dan Carpenter
---
>From static analysis. Not tested.
drivers/mtd/nand/raw/nand_base.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index fb072c95..d83c0503f96f 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -880,6 +880,7 @@ static int nand_setup_interface(struct nand_chip *chip, int
chipnr)
if (tmode_param[0] != chip->best_interface_config->timings.mode) {
pr_warn("timing mode %d not acknowledged by the NAND chip\n",
chip->best_interface_config->timings.mode);
+ ret = -EINVAL;
goto err_reset_chip;
}
--
2.30.2
Re: [PATCH net-next v2 1/2] of: net: pass the dst buffer to of_get_mac_address()
Hi Michael,
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/of-net-support-non-platform-devices-in-of_get_mac_address/20210406-234030
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
cc0626c2aaed8e475efdd85fa374b497a7192e35
config: x86_64-randconfig-m001-20210406 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/net/ethernet/xilinx/xilinx_axienet_main.c:2069 axienet_probe() warn:
passing a valid pointer to 'PTR_ERR'
vim +/PTR_ERR +2069 drivers/net/ethernet/xilinx/xilinx_axienet_main.c
522856cefaf09d Robert Hancock 2019-06-06 2060 /* Check for Ethernet
core IRQ (optional) */
522856cefaf09d Robert Hancock 2019-06-06 2061 if (lp->eth_irq <= 0)
522856cefaf09d Robert Hancock 2019-06-06 2062
dev_info(>dev, "Ethernet core IRQ not defined\n");
522856cefaf09d Robert Hancock 2019-06-06 2063
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2064 /* Retrieve the MAC
address */
411b125c6ace1f Michael Walle 2021-04-06 2065 ret =
of_get_mac_address(pdev->dev.of_node, mac_addr);
411b125c6ace1f Michael Walle 2021-04-06 2066 if (!ret) {
411b125c6ace1f Michael Walle 2021-04-06 2067
axienet_set_mac_address(ndev, mac_addr);
411b125c6ace1f Michael Walle 2021-04-06 2068 } else {
d05a9ed5c3a773 Robert Hancock 2019-06-06 @2069
dev_warn(>dev, "could not find MAC address property: %ld\n",
d05a9ed5c3a773 Robert Hancock 2019-06-06 2070
PTR_ERR(mac_addr));
^
This should print "ret".
411b125c6ace1f Michael Walle 2021-04-06 2071
axienet_set_mac_address(ndev, NULL);
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2072 }
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2073
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2074 lp->coalesce_count_rx =
XAXIDMA_DFT_RX_THRESHOLD;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2075 lp->coalesce_count_tx =
XAXIDMA_DFT_TX_THRESHOLD;
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Tue, Apr 13, 2021 at 10:08:32PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, April 13, 2021 9:48:44 PM CEST Matthew Wilcox wrote:
> > On Tue, Apr 13, 2021 at 09:45:03PM +0200, Fabio M. De Francesco wrote:
> > > 1) The driver doesn't call that function from anywhere else than the
> > > macro. 2) You have explained that the macro add its symbol to a slot
> > > in an array that would shift all the subsequent elements down if that
> > > macro is not used exactly in the line where it is.
> > > 3) Dan Carpenter said that that array is full of null functions (or
> > > empty slots?).
> > >
> > > Unless that function is called anonymously dereferencing its address
> > > from the position it occupies in the array, I'm not able to see what
> > > else means can any caller use.
> > >
> > > I know I have much less experience than you with C: what can go wrong?
> >
> > Here's where the driver calls that function:
> >
> > $ git grep wlancmds drivers/staging/rtl8723bs/
> > drivers/staging/rtl8723bs/core/rtw_cmd.c:static struct cmd_hdl wlancmds[]
> > = { drivers/staging/rtl8723bs/core/rtw_cmd.c: if
> > (pcmd->cmdcode < ARRAY_SIZE(wlancmds)) {
> > drivers/staging/rtl8723bs/core/rtw_cmd.c: cmd_hdl
> > = wlancmds[pcmd->cmdcode].h2cfuns;
> >
> OK, I had imagined an anonymous call from its location in the array (as I
> wrote in the last phrase of my message). However, I thought that it could
> have been an improbable possibility, not a real one.
>
> Linux uses a lot of interesting ideas that newcomers like me should learn.
> Things here are trickier than they appear at first sight.
One trick would be to build the Smatch cross function database.
https://www.spinics.net/lists/smatch/msg00568.html
Then you could do:
$ ~/path/to/smatch_data/db/smdb.py led_blink_hdl
file | caller | function | type | parameter | key | value |
drivers/staging/rtl8723bs/core/rtw_cmd.c | rtw_cmd_thread |
rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
uchar(*)(struct adapter*, uchar*)
drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
rtw_cmd_thread ptr cmd_hdl | INTERNAL | -1 | |
uchar(*)(struct adapter*, uchar*)
drivers/staging/rtl8188eu/core/rtw_cmd.c | rtw_cmd_thread |
rtw_cmd_thread ptr cmd_hdl | BUF_SIZE | 1 |pbuf |
1,4,6,8,12,14,16,19-20,23-24,48,740,884,892,900,960
Which says that led_blink_hdl() is called as a function pointer called
"cmd_hdl" from rtw_cmd_thread().
Hm... It says it can be called from either rtw_cmd_thread() function
(the rtl8723bs or rtl8188eu version) which is not ideal. But also
basically harmless so whatever...
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: hal: Remove four set but not used variables
On Tue, Apr 13, 2021 at 08:42:22PM +0200, Fabio M. De Francesco wrote:
> Removed four variables that were set but not used.
>
> Signed-off-by: Fabio M. De Francesco
> ---
> drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index 77f8353c5ce5..fad6a3bfe07c 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -3199,12 +3199,10 @@ static void hw_var_set_mlme_join(struct adapter
> *padapter, u8 variable, u8 *val)
>
> void CCX_FwC2HTxRpt_8723b(struct adapter *padapter, u8 *pdata, u8 len)
> {
> - u8 seq_no;
>
> #define GET_8723B_C2H_TX_RPT_LIFE_TIME_OVER(_Header)
> LE_BITS_TO_1BYTE((_Header + 0), 6, 1)
> #define GET_8723B_C2H_TX_RPT_RETRY_OVER(_Header)
> LE_BITS_TO_1BYTE((_Header + 0), 7, 1)
>
> - seq_no = *(pdata+6);
>
> if (GET_8723B_C2H_TX_RPT_RETRY_OVER(pdata) |
> GET_8723B_C2H_TX_RPT_LIFE_TIME_OVER(pdata)) {
Now we have two blank lines in a row. Please delete one.
regards,
dan carpenter
Re: [syzbot] WARNING in unsafe_follow_pfn
On Tue, Apr 13, 2021 at 03:11:45PM -0300, Jason Gunthorpe wrote:
> On Tue, Apr 13, 2021 at 07:20:12PM +0200, Dmitry Vyukov wrote:
> > > > Plus users are going to be seeing this as well. According to the commit
> > > > message for 69bacee7f9ad ("mm: Add unsafe_follow_pfn") "Unfortunately
> > > > there's some users where this is not fixable (like v4l userptr of iomem
> > > > mappings)". It sort of seems crazy to dump this giant splat and then
> > > > tell users to ignore it forever because it can't be fixed... 0_0
> > >
> > > I think the discussion conclusion was that this interface should not
> > > be used by userspace anymore, it is obsolete by some new interface?
> > >
> > > It should be protected by some kconfig and the kconfig should be
> > > turned off for syzkaller runs.
> >
> > If this is not a kernel bug, then it must not use WARN_ON[_ONCE]. It
> > makes the kernel untestable for both automated systems and humans:
>
> It is a kernel security bug triggerable by userspace.
>
> > And if it's a kernel bug reachable from user-space, then I think this
> > code should be removed entirely, not just on all testing systems. Or
> > otherwise if we are not removing it for some reason, then it needs to
> > be fixed.
>
> Legacy embedded systems apparently require it.
Are legacy embedded systems ever going to update their kernel? It might
be better to just remove it. (I don't really have any details outside
of your email so I don't know).
regards,
dan carpenter
Re: [Outreachy kernel] [PATCH] :staging: rtl8723bs: Remove useless led_blink_hdl()
On Tue, Apr 13, 2021 at 06:47:06PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, April 13, 2021 6:27:17 PM CEST Julia Lawall wrote:
> > On Tue, 13 Apr 2021, Fabio M. De Francesco wrote:
> > > On Tuesday, April 13, 2021 6:04:16 PM CEST Julia Lawall wrote:
> > > > On Tue, 13 Apr 2021, Fabio M. De Francesco wrote:
> > > > > Removed the led_blink_hdl() function (declaration, definition, and
> > > > > caller code) because it's useless. It only seems to check whether
> > > > > or
> > > > > not a given pointer is NULL. There are other (simpler) means for
> > > > > that
> > > > > purpose.
> > > > >
> > > > > Signed-off-by: Fabio M. De Francesco
> > > > > ---
> > > > >
> > > > > drivers/staging/rtl8723bs/core/rtw_cmd.c | 1 -
> > > > > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c| 9 -
> > > > > drivers/staging/rtl8723bs/include/rtw_mlme_ext.h | 1 -
> > > > > 3 files changed, 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > > > > b/drivers/staging/rtl8723bs/core/rtw_cmd.c index
> > > > > 0297fbad7bce..4c44dfd21514 100644
> > > > > --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > > > > +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > > > > @@ -150,7 +150,6 @@ static struct cmd_hdl wlancmds[] = {
> > > > >
> > > > > GEN_MLME_EXT_HANDLER(0, h2c_msg_hdl) /*58*/
> > > > > GEN_MLME_EXT_HANDLER(sizeof(struct SetChannelPlan_param),
> > > > > set_chplan_hdl) /*59*/>
> > > > >
> > > > > - GEN_MLME_EXT_HANDLER(sizeof(struct LedBlink_param),
> > >
> > > led_blink_hdl)
> > >
> > > > > /*60*/
> > > >
> > > > This is worrisome. Doyou fully understand the impact of this? If
> > > > not,
> > > > the change is probably not a good idea.
> > >
> > > This is that macro definition:
> > >
> > > #define GEN_MLME_EXT_HANDLER(size, cmd) {size, cmd},
> > >
> > > struct C2HEvent_Header {
> > >
> > > #ifdef __LITTLE_ENDIAN
> > >
> > > unsigned int len:16;
> > > unsigned int ID:8;
> > > unsigned int seq:8;
> > >
> > > #else
> > >
> > > unsigned int seq:8;
> > > unsigned int ID:8;
> > > unsigned int len:16;
> > >
> > > #endif
> > >
> > > unsigned int rsvd;
> > >
> > > };
> > >
> > > It's a bit convoluted with regard to my experience. Probably I don't
> > > understand it fully, but it seems to me to not having effects to the
> > > code where I removed its use within core/rtw_cmd.c.
> > >
> > > What am I missing?
> >
> > It seems that the function is being put into an array. Probably someone
> > expects to find it there. Probably you have shifted all of the functions
> > that come afterwards back one slot so that they are all in the wrong
> > places.
> >
> > julia
> >
> Thanks for your explanation. Obviously this implies that the function
> cannot be removed, unless one fill the slot that is deleted by to not
> calling this macro at the right moment.
>
> I also suppose that providing a function pointer with a NULL value wouldn't
> work either.
It would work. That array is full of NULL function pointers.
regards,
dan carpenter
Re: [PATCH v3 4/4] staging: media: intel-ipu3: remove space before tabs
On Tue, Apr 13, 2021 at 08:59:34PM +0530, Mitali Borkar wrote:
> Removed unnecessary space before tabs to adhere to linux kernel coding
> style.
> Reported by checkpatch.
>
> Signed-off-by: Mitali Borkar
> ---
>
> Changes from v2:- No changes.
> Changes from v1:- No changes.
>
> drivers/staging/media/ipu3/include/intel-ipu3.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/ipu3/include/intel-ipu3.h
> b/drivers/staging/media/ipu3/include/intel-ipu3.h
> index 47e98979683c..42edac5ee4e4 100644
> --- a/drivers/staging/media/ipu3/include/intel-ipu3.h
> +++ b/drivers/staging/media/ipu3/include/intel-ipu3.h
> @@ -633,7 +633,7 @@ struct ipu3_uapi_bnr_static_config_wb_gains_thr_config {
> * @cg: Gain coefficient for threshold calculation, [0, 31], default 8.
> * @ci: Intensity coefficient for threshold calculation. range [0, 0x1f]
> * default 6.
> - * format: u3.2 (3 most significant bits represent whole number,
> + *format: u3.2 (3 most significant bits represent whole number,
> * 2 least significant bits represent the fractional part
Just remove the spaces, don't remove the tab. It's looks silly now.
regards,
dan carpenter
Re: [PATCH 18/19] staging: rtl8723bs: split long lines
On Wed, Apr 07, 2021 at 03:49:42PM +0200, Fabio Aiuto wrote:
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 7f998a2f8001..a1e27ba4707e 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -619,8 +619,9 @@ unsigned int OnProbeReq(struct adapter *padapter, union
> recv_frame *precv_frame)
> return _SUCCESS;
>
> _issue_probersp:
> - if ((check_fwstate(pmlmepriv, _FW_LINKED) &&
> - pmlmepriv->cur_network.join_res) ||
> check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
> + if ((check_fwstate(pmlmepriv, _FW_LINKED) &&
> + pmlmepriv->cur_network.join_res) ||
> + check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
The last line is indented one space character too much. It should be:
if ((check_fwstate(pmlmepriv, _FW_LINKED) &&
pmlmepriv->cur_network.join_res) ||
check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE))
regards,
dan carpenter
Re: [PATCH 12/19] staging: rtl8723bs: remove unnecessary bracks on DBG_871X removal sites
On Wed, Apr 07, 2021 at 03:49:36PM +0200, Fabio Aiuto wrote:
> @@ -2586,11 +2583,9 @@ static int rtw_dbg_port(struct net_device *dev,
>
> plist =
> get_next(plist);
>
> - if (extra_arg
> == psta->aid) {
> - for (j
> = 0; j < 16; j++) {
> + if (extra_arg
> == psta->aid)
> + for (j
> = 0; j < 16; j++)
>
> preorder_ctrl = >recvreorder_ctrl[j];
> - }
> - }
I think Greg already applied this so no stress (don't bother fixing),
but you removed a bit too much on this one. Multi-line indents normally
get curly braces for readability. In other words:
if (extra_arg == psta->aid) {
for (j = 0; j < 16; j++)
preorder_ctrl =
>recvreorder_ctrl[j];
}
regards,
dan carpenter
Re: [PATCH 3/4] staging: rtl8712: remove space after cast
This patch has already been applied, which is fine, but really all these casts are totally pointless and should be removed. regards, dan carpenter
[PATCH] Drivers: hv: vmbus: Use after free in __vmbus_open()
The "open_info" variable is added to the _connection.chn_msg_list,
but the error handling frees "open_info" without removing it from the
list. This will result in a use after free. First remove it from the
list, and then free it.
Fixes: 6f3d791f3006 ("Drivers: hv: vmbus: Fix rescind handling issues")
Signed-off-by: Dan Carpenter
---
>From static analysis. Untested etc. There is almost certainly a good
reason to add it to the list before checking "newchannel->rescind" but I
don't know the code well enough to know what the reason is.
drivers/hv/channel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index db30be8f9cce..1c5a418c1962 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -653,7 +653,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
if (newchannel->rescind) {
err = -ENODEV;
- goto error_free_info;
+ goto error_clean_msglist;
}
err = vmbus_post_msg(open_msg,
--
2.30.2
Re: [PATCH] Staging: rtl8192u: ieee80211: fixed a trailing statements coding style issue.
On Sun, Apr 04, 2021 at 08:15:00AM +0300, dev.dra...@bk.ru wrote:
> From: Dmitrii Wolf
>
> Signed-off-by: Dmitrii Wolf
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index 690b664df8fa..29a6ce20e2bd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -2048,12 +2048,12 @@ void ieee80211_softmac_xmit(struct ieee80211_txb
> *txb, struct ieee80211_device *
> /* if xmit available, just xmit it immediately, else just insert it to
> the wait queue */
> for (i = 0; i < txb->nr_frags; i++) {
> #ifdef USB_TX_DRIVER_AGGREGATION_ENABLE
> - if ((skb_queue_len(>skb_drv_aggQ[queue_index]) != 0) ||
> + if ((skb_queue_len(>skb_drv_aggQ[queue_index]) != 0)
> #else
> - if ((skb_queue_len(>skb_waitQ[queue_index]) != 0) ||
> + if ((skb_queue_len(>skb_waitQ[queue_index]) != 0)
> #endif
> - (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) || \
> - (ieee->queue_stop)) {
> + || (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) \
> + || (ieee->queue_stop)) {
No. The || go on the end, the way the original code had it. I've no
idea what you were trying to fix. Maybe the \ characters?
regards,
dan carpenter
[PATCH] HSI: core: fix resource leaks in hsi_add_client_from_dt()
If some of the allocations fail between the dev_set_name() and the
device_register() then the name will not be freed. Fix this by
moving dev_set_name() directly in front of the call to device_register().
Fixes: a2aa24734d9d ("HSI: Add common DT binding for HSI client devices")
Signed-off-by: Dan Carpenter
---
Jason, this is the most common type of error I see with device_register().
Is there a downside to calling dev_set_name() later? Presumably it's
printed out somewhere, but I feel like just moving the dev_set_name() is
almost always the best and simplest fix.
drivers/hsi/hsi_core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/hsi/hsi_core.c b/drivers/hsi/hsi_core.c
index c3fb5beb846e..ec90713564e3 100644
--- a/drivers/hsi/hsi_core.c
+++ b/drivers/hsi/hsi_core.c
@@ -210,8 +210,6 @@ static void hsi_add_client_from_dt(struct hsi_port *port,
if (err)
goto err;
- dev_set_name(>device, "%s", name);
-
err = hsi_of_property_parse_mode(client, "hsi-mode", );
if (err) {
err = hsi_of_property_parse_mode(client, "hsi-rx-mode",
@@ -293,6 +291,7 @@ static void hsi_add_client_from_dt(struct hsi_port *port,
cl->device.release = hsi_client_release;
cl->device.of_node = client;
+ dev_set_name(>device, "%s", name);
if (device_register(>device) < 0) {
pr_err("hsi: failed to register client: %s\n", name);
put_device(>device);
--
2.30.2
[PATCH] node: fix device cleanups in error handling code
We can't use kfree() to free device managed resources so the kfree(dev)
is against the rules.
It's easier to write this code if we open code the device_register() as
a device_initialize() and device_add(). That way if dev_set_name() set
name fails we can call put_device() and it will clean up correctly.
Fixes: acc02a109b04 ("node: Add memory-side caching attributes")
Signed-off-by: Dan Carpenter
---
drivers/base/node.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index f449dbb2c746..2c36f61d30bc 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -268,21 +268,20 @@ static void node_init_cache_dev(struct node *node)
if (!dev)
return;
+ device_initialize(dev);
dev->parent = >dev;
dev->release = node_cache_release;
if (dev_set_name(dev, "memory_side_cache"))
- goto free_dev;
+ goto put_device;
- if (device_register(dev))
- goto free_name;
+ if (device_add(dev))
+ goto put_device;
pm_runtime_no_callbacks(dev);
node->cache_dev = dev;
return;
-free_name:
- kfree_const(dev->kobj.name);
-free_dev:
- kfree(dev);
+put_device:
+ put_device(dev);
}
/**
@@ -319,25 +318,24 @@ void node_add_cache(unsigned int nid, struct
node_cache_attrs *cache_attrs)
return;
dev = >dev;
+ device_initialize(dev);
dev->parent = node->cache_dev;
dev->release = node_cacheinfo_release;
dev->groups = cache_groups;
if (dev_set_name(dev, "index%d", cache_attrs->level))
- goto free_cache;
+ goto put_device;
info->cache_attrs = *cache_attrs;
- if (device_register(dev)) {
+ if (device_add(dev)) {
dev_warn(>dev, "failed to add cache level:%d\n",
cache_attrs->level);
- goto free_name;
+ goto put_device;
}
pm_runtime_no_callbacks(dev);
list_add_tail(>node, >cache_attrs);
return;
-free_name:
- kfree_const(dev->kobj.name);
-free_cache:
- kfree(info);
+put_device:
+ put_device(dev);
}
static void node_remove_caches(struct node *node)
--
2.30.2
Re: [PATCH 02/10] staging: rtl8723bs: remove commented out RT_TRACE logs in hal/ and os_dep/
On Mon, Apr 05, 2021 at 06:49:49PM +0200, Fabio Aiuto wrote:
> diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> index 9b16265b543d..f52cc4e7a6e6 100644
> --- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> +++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> @@ -1785,7 +1785,6 @@ s8 phy_get_tx_pwr_lmt(struct adapter *adapter, u32
> reg_pwr_tbl_sel,
> /*
> if (band_type == BAND_ON_5G && pwr_lmt == MAX_POWER_INDEX) {
> if (idx_bandwidth == 0 || idx_bandwidth == 1) {
> - RT_TRACE(COMP_INIT, DBG_LOUD, ("No power limit table of
> the specified band %d, bandwidth %d, ratesection %d, rf path %d\n",
>idx_band, idx_bandwidth,
>idx_rate_sctn, rf_path));
> if (idx_rate_sctn == 2)
This won't compile.
regards,
dan carpenter
[kbuild] Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
Hi,
url:
https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-m021-20210407 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/of/overlay.c:1045 of_overlay_fdt_apply() warn: overwrite may leak
'new_fdt'
vim +/new_fdt +1045 drivers/of/overlay.c
39a751a4cb7e47 Frank Rowand 2018-02-12 1015 int
of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
39a751a4cb7e47 Frank Rowand 2018-02-12 1016int
*ovcs_id)
39a751a4cb7e47 Frank Rowand 2018-02-12 1017 {
7a18fbf9013a19 Frank Rowand 2021-04-07 1018 void *new_fdt;
39a751a4cb7e47 Frank Rowand 2018-02-12 1019 int ret;
39a751a4cb7e47 Frank Rowand 2018-02-12 1020 u32 size;
39a751a4cb7e47 Frank Rowand 2018-02-12 1021 struct device_node
*overlay_root;
39a751a4cb7e47 Frank Rowand 2018-02-12 1022
39a751a4cb7e47 Frank Rowand 2018-02-12 1023 *ovcs_id = 0;
39a751a4cb7e47 Frank Rowand 2018-02-12 1024 ret = 0;
39a751a4cb7e47 Frank Rowand 2018-02-12 1025
39a751a4cb7e47 Frank Rowand 2018-02-12 1026 if (overlay_fdt_size <
sizeof(struct fdt_header) ||
39a751a4cb7e47 Frank Rowand 2018-02-12 1027
fdt_check_header(overlay_fdt)) {
39a751a4cb7e47 Frank Rowand 2018-02-12 1028 pr_err("Invalid
overlay_fdt header\n");
39a751a4cb7e47 Frank Rowand 2018-02-12 1029 return -EINVAL;
39a751a4cb7e47 Frank Rowand 2018-02-12 1030 }
39a751a4cb7e47 Frank Rowand 2018-02-12 1031
39a751a4cb7e47 Frank Rowand 2018-02-12 1032 size =
fdt_totalsize(overlay_fdt);
39a751a4cb7e47 Frank Rowand 2018-02-12 1033 if (overlay_fdt_size <
size)
39a751a4cb7e47 Frank Rowand 2018-02-12 1034 return -EINVAL;
39a751a4cb7e47 Frank Rowand 2018-02-12 1035
39a751a4cb7e47 Frank Rowand 2018-02-12 1036 /*
39a751a4cb7e47 Frank Rowand 2018-02-12 1037* Must create
permanent copy of FDT because of_fdt_unflatten_tree()
39a751a4cb7e47 Frank Rowand 2018-02-12 1038* will create pointers
to the passed in FDT in the unflattened tree.
39a751a4cb7e47 Frank Rowand 2018-02-12 1039*/
7a18fbf9013a19 Frank Rowand 2021-04-07 1040 size += FDT_ALIGN_SIZE;
7a18fbf9013a19 Frank Rowand 2021-04-07 1041 new_fdt = kmalloc(size,
GFP_KERNEL);
39a751a4cb7e47 Frank Rowand 2018-02-12 1042 if (!new_fdt)
39a751a4cb7e47 Frank Rowand 2018-02-12 1043 return -ENOMEM;
39a751a4cb7e47 Frank Rowand 2018-02-12 1044
7a18fbf9013a19 Frank Rowand 2021-04-07 @1045 new_fdt =
PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
^^^
We're not freeing the exact same pointer that we allocated.
7a18fbf9013a19 Frank Rowand 2021-04-07 1046 memcpy(new_fdt,
overlay_fdt, size);
7a18fbf9013a19 Frank Rowand 2021-04-07 1047
39a751a4cb7e47 Frank Rowand 2018-02-12 1048
of_fdt_unflatten_tree(new_fdt, NULL, _root);
39a751a4cb7e47 Frank Rowand 2018-02-12 1049 if (!overlay_root) {
39a751a4cb7e47 Frank Rowand 2018-02-12 1050 pr_err("unable
to unflatten overlay_fdt\n");
39a751a4cb7e47 Frank Rowand 2018-02-12 1051 ret = -EINVAL;
39a751a4cb7e47 Frank Rowand 2018-02-12 1052 goto
out_free_new_fdt;
39a751a4cb7e47 Frank Rowand 2018-02-12 1053 }
39a751a4cb7e47 Frank Rowand 2018-02-12 1054
39a751a4cb7e47 Frank Rowand 2018-02-12 1055 ret =
of_overlay_apply(new_fdt, overlay_root, ovcs_id);
39a751a4cb7e47 Frank Rowand 2018-02-12 1056 if (ret < 0) {
39a751a4cb7e47 Frank Rowand 2018-02-12 1057 /*
39a751a4cb7e47 Frank Rowand 2018-02-12 1058* new_fdt and
overlay_root now belong to the overlay
39a751a4cb7e47 Frank Rowand 2018-02-12 1059* changeset.
39a751a4cb7e47 Frank Rowand 2018-02-12 1060* overlay
changeset code is responsible for freeing them.
39a751a4cb7e47 Frank Rowand 2018-02-12 1061*/
39a751a4cb7e47 Frank Rowand 2018-02-12 1062 goto out;
39a751a4cb7e47 Frank Rowand 2018-02-12 1063 }
39a751a4cb7e47 Frank Rowand 2018-02-12 1064
39a751a4cb7e47 Frank Rowand 2018-02-12 1065 return 0;
39a751a4cb7e47 Frank Rowand 2018-02-12 1066
39a751a4cb7e47 Frank Rowand 2018-02-12 1067
39a751a4cb7e47 Frank Rowand 2018-02-12 1068 out_free_new_fdt:
39a751a4
[kbuild] drivers/gpu/drm/msm/adreno/a3xx_gpu.c:600 a3xx_gpu_init() error: passing non negative 1 to ERR_PTR
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: 2d743660786ec51f5c1fefd5782bbdee7b227db0
commit: 5785dd7a8ef0de8049f40a1a109de6a1bf17b479 drm/msm: Fix duplicate gpu
node in icc summary
config: arm64-randconfig-m031-20210407 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/msm/adreno/a3xx_gpu.c:600 a3xx_gpu_init() error: passing non
negative 1 to ERR_PTR
drivers/gpu/drm/msm/adreno/a4xx_gpu.c:727 a4xx_gpu_init() error: passing non
negative 1 to ERR_PTR
vim +600 drivers/gpu/drm/msm/adreno/a3xx_gpu.c
7198e6b03155f6 Rob Clark 2013-07-19 515 struct msm_gpu
*a3xx_gpu_init(struct drm_device *dev)
7198e6b03155f6 Rob Clark 2013-07-19 516 {
7198e6b03155f6 Rob Clark 2013-07-19 517 struct a3xx_gpu *a3xx_gpu =
NULL;
55459968176f13 Rob Clark 2013-12-05 518 struct adreno_gpu *adreno_gpu;
7198e6b03155f6 Rob Clark 2013-07-19 519 struct msm_gpu *gpu;
060530f1ea6740 Rob Clark 2014-03-03 520 struct msm_drm_private *priv =
dev->dev_private;
060530f1ea6740 Rob Clark 2014-03-03 521 struct platform_device *pdev =
priv->gpu_pdev;
5785dd7a8ef0de Akhil P Oommen 2020-10-28 522 struct icc_path *ocmem_icc_path;
5785dd7a8ef0de Akhil P Oommen 2020-10-28 523 struct icc_path *icc_path;
7198e6b03155f6 Rob Clark 2013-07-19 524 int ret;
7198e6b03155f6 Rob Clark 2013-07-19 525
7198e6b03155f6 Rob Clark 2013-07-19 526 if (!pdev) {
6a41da17e87dee Mamta Shukla 2018-10-20 527 DRM_DEV_ERROR(dev->dev,
"no a3xx device\n");
7198e6b03155f6 Rob Clark 2013-07-19 528 ret = -ENXIO;
7198e6b03155f6 Rob Clark 2013-07-19 529 goto fail;
7198e6b03155f6 Rob Clark 2013-07-19 530 }
7198e6b03155f6 Rob Clark 2013-07-19 531
7198e6b03155f6 Rob Clark 2013-07-19 532 a3xx_gpu =
kzalloc(sizeof(*a3xx_gpu), GFP_KERNEL);
7198e6b03155f6 Rob Clark 2013-07-19 533 if (!a3xx_gpu) {
7198e6b03155f6 Rob Clark 2013-07-19 534 ret = -ENOMEM;
7198e6b03155f6 Rob Clark 2013-07-19 535 goto fail;
7198e6b03155f6 Rob Clark 2013-07-19 536 }
7198e6b03155f6 Rob Clark 2013-07-19 537
55459968176f13 Rob Clark 2013-12-05 538 adreno_gpu = _gpu->base;
55459968176f13 Rob Clark 2013-12-05 539 gpu = _gpu->base;
7198e6b03155f6 Rob Clark 2013-07-19 540
70c70f091b1ffd Rob Clark 2014-05-30 541 gpu->perfcntrs = perfcntrs;
70c70f091b1ffd Rob Clark 2014-05-30 542 gpu->num_perfcntrs =
ARRAY_SIZE(perfcntrs);
70c70f091b1ffd Rob Clark 2014-05-30 543
3bcefb0497f9fc Rob Clark 2014-09-05 544 adreno_gpu->registers =
a3xx_registers;
3bcefb0497f9fc Rob Clark 2014-09-05 545
f97decac5f4c2d Jordan Crouse 2017-10-20 546 ret = adreno_gpu_init(dev,
pdev, adreno_gpu, , 1);
7198e6b03155f6 Rob Clark 2013-07-19 547 if (ret)
7198e6b03155f6 Rob Clark 2013-07-19 548 goto fail;
7198e6b03155f6 Rob Clark 2013-07-19 549
55459968176f13 Rob Clark 2013-12-05 550 /* if needed, allocate gmem: */
55459968176f13 Rob Clark 2013-12-05 551 if (adreno_is_a330(adreno_gpu))
{
26c0b26dcd005d Brian Masney 2019-08-23 552 ret =
adreno_gpu_ocmem_init(_gpu->base.pdev->dev,
26c0b26dcd005d Brian Masney 2019-08-23 553
adreno_gpu, _gpu->ocmem);
26c0b26dcd005d Brian Masney 2019-08-23 554 if (ret)
26c0b26dcd005d Brian Masney 2019-08-23 555 goto fail;
55459968176f13 Rob Clark 2013-12-05 556 }
55459968176f13 Rob Clark 2013-12-05 557
667ce33e57d0de Rob Clark 2016-09-28 558 if (!gpu->aspace) {
871d812aa43e63 Rob Clark 2013-11-16 559 /* TODO we think it is
possible to configure the GPU to
871d812aa43e63 Rob Clark 2013-11-16 560* restrict access to
VRAM carveout. But the required
871d812aa43e63 Rob Clark 2013-11-16 561* registers are
unknown. For now just bail out and
871d812aa43e63 Rob Clark 2013-11-16 562* limp along with just
modesetting. If it turns out
871d812aa43e63 Rob Clark 2013-11-16 563* to not be possible
to restrict access, then we must
871d812aa43e63 Rob Clark 2013-11-16 564* implement a
cmdstream validator.
871d812aa43e63 Rob Clark 2013-11-16 565*/
6a41da17e87dee Mamta Shukla 2018-10-20 566 DRM_DEV_ERROR(dev->dev,
"No memory protection without IOMMU\n");
871d812aa43e63 Rob Clark 2013-11-16 567 ret = -ENXIO;
871d812aa43e63 Rob Clark 2013-11-16 568 goto fail;
871d812aa43e63 Rob Clark 2013-11-16 569 }
871d812aa43e63 Rob Clark 2013-11-16 570
[kbuild] Re: [PATCH net-next v2 1/2] of: net: pass the dst buffer to of_get_mac_address()
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/of-net-support-non-platform-devices-in-of_get_mac_address/20210406-234030
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
cc0626c2aaed8e475efdd85fa374b497a7192e35
config: x86_64-randconfig-m001-20210406 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/net/ethernet/xilinx/xilinx_axienet_main.c:2069 axienet_probe() warn:
passing a valid pointer to 'PTR_ERR'
vim +/PTR_ERR +2069 drivers/net/ethernet/xilinx/xilinx_axienet_main.c
2be586205ca2b8 Srikanth Thokala2015-05-05 1832 static int
axienet_probe(struct platform_device *pdev)
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1833 {
8495659bf93c8e Srikanth Thokala2015-05-05 1834 int ret;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1835 struct device_node *np;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1836 struct axienet_local
*lp;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1837 struct net_device *ndev;
28ef9ebdb64c6f Robert Hancock 2019-06-06 1838 struct resource *ethres;
411b125c6ace1f Michael Walle 2021-04-06 1839 u8 mac_addr[ETH_ALEN];
^^
5fff0151b3244d Andre Przywara 2020-03-24 1840 int addr_width = 32;
8495659bf93c8e Srikanth Thokala2015-05-05 1841 u32 value;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1842
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1843 ndev =
alloc_etherdev(sizeof(*lp));
41de8d4cff21a2 Joe Perches 2012-01-29 1844 if (!ndev)
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1845 return -ENOMEM;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1846
95219aa538e11d Srikanth Thokala2015-05-05 1847
platform_set_drvdata(pdev, ndev);
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1848
95219aa538e11d Srikanth Thokala2015-05-05 1849 SET_NETDEV_DEV(ndev,
>dev);
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1850 ndev->flags &=
~IFF_MULTICAST; /* clear multicast */
28e24c62ab3062 Eric Dumazet2013-12-02 1851 ndev->features =
NETIF_F_SG;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1852 ndev->netdev_ops =
_netdev_ops;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1853 ndev->ethtool_ops =
_ethtool_ops;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1854
d894be57ca92c8 Jarod Wilson2016-10-20 1855 /* MTU range: 64 - 9000
*/
d894be57ca92c8 Jarod Wilson2016-10-20 1856 ndev->min_mtu = 64;
d894be57ca92c8 Jarod Wilson2016-10-20 1857 ndev->max_mtu =
XAE_JUMBO_MTU;
d894be57ca92c8 Jarod Wilson2016-10-20 1858
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1859 lp = netdev_priv(ndev);
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1860 lp->ndev = ndev;
95219aa538e11d Srikanth Thokala2015-05-05 1861 lp->dev = >dev;
8a3b7a252dca9f Daniel Borkmann 2012-01-19 1862 lp->options =
XAE_OPTION_DEFAULTS;
8b09ca823ffb4e Robert Hancock 2019-06-06 1863 lp->rx_bd_num =
RX_BD_NUM_DEFAULT;
8b09ca823ffb4e Robert Hancock 2019-06-06 1864 lp->tx_bd_num =
TX_BD_NUM_DEFAULT;
57baf8cc70ea4c Robert Hancock 2021-02-12 1865
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1866 lp->axi_clk =
devm_clk_get_optional(>dev, "s_axi_lite_clk");
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1867 if (!lp->axi_clk) {
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1868 /* For backward
compatibility, if named AXI clock is not present,
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1869 * treat the
first clock specified as the AXI clock.
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1870 */
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1871 lp->axi_clk =
devm_clk_get_optional(>dev, NULL);
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1872 }
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1873 if
(IS_ERR(lp->axi_clk)) {
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1874 ret =
PTR_ERR(lp->axi_clk);
57baf8cc70ea4c Robert Hancock 2021-02-12 1875 goto
free_netdev;
57baf8cc70ea4c Robert Hancock 2021-02-12 1876 }
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1877 ret =
clk_prepare_enable(lp->axi_clk);
57baf8cc70ea4c Robert Hancock 2021-02-12 1878 if (ret) {
b11bfb9a19f9d7 Robert Hancock 2021-03-25 1879
dev_err(>dev, "Unable to enable AXI clock: %d\n", ret);
57baf8cc70ea4c Robert Hancock 2021-02-12 1880 goto
free_netdev;
57baf8cc70ea4c Robe
[kbuild] drivers/vdpa/mlx5/core/mr.c:282 map_direct_mr() warn: missing error code 'err'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: e49d033bddf5b565044e2abe4241353959bc9120
commit: 94abbccdf2916cb03f9626f2d36c6e9971490c12 vdpa/mlx5: Add shared memory
registration code
config: microblaze-randconfig-m031-20210405 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/vdpa/mlx5/core/mr.c:282 map_direct_mr() warn: missing error code 'err'
Old smatch warnings:
drivers/vdpa/mlx5/core/mr.c:350 add_direct_chain() error: uninitialized symbol
'err'.
drivers/vdpa/mlx5/core/mr.c:483 mlx5_vdpa_handle_set_map() error: uninitialized
symbol 'err'.
vim +/err +282 drivers/vdpa/mlx5/core/mr.c
94abbccdf2916c Eli Cohen 2020-08-04 226 static int map_direct_mr(struct
mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr,
94abbccdf2916c Eli Cohen 2020-08-04 227 struct
vhost_iotlb *iotlb)
94abbccdf2916c Eli Cohen 2020-08-04 228 {
94abbccdf2916c Eli Cohen 2020-08-04 229struct vhost_iotlb_map *map;
94abbccdf2916c Eli Cohen 2020-08-04 230unsigned long lgcd = 0;
94abbccdf2916c Eli Cohen 2020-08-04 231int log_entity_size;
94abbccdf2916c Eli Cohen 2020-08-04 232unsigned long size;
94abbccdf2916c Eli Cohen 2020-08-04 233u64 start = 0;
94abbccdf2916c Eli Cohen 2020-08-04 234int err;
94abbccdf2916c Eli Cohen 2020-08-04 235struct page *pg;
94abbccdf2916c Eli Cohen 2020-08-04 236unsigned int nsg;
94abbccdf2916c Eli Cohen 2020-08-04 237int sglen;
94abbccdf2916c Eli Cohen 2020-08-04 238u64 pa;
94abbccdf2916c Eli Cohen 2020-08-04 239u64 paend;
94abbccdf2916c Eli Cohen 2020-08-04 240struct scatterlist *sg;
94abbccdf2916c Eli Cohen 2020-08-04 241struct device *dma =
mvdev->mdev->device;
94abbccdf2916c Eli Cohen 2020-08-04 242int ret;
94abbccdf2916c Eli Cohen 2020-08-04 243
94abbccdf2916c Eli Cohen 2020-08-04 244for (map =
vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
94abbccdf2916c Eli Cohen 2020-08-04 245 map; map =
vhost_iotlb_itree_next(map, start, mr->end - 1)) {
94abbccdf2916c Eli Cohen 2020-08-04 246size = maplen(map, mr);
94abbccdf2916c Eli Cohen 2020-08-04 247lgcd = gcd(lgcd, size);
94abbccdf2916c Eli Cohen 2020-08-04 248start += size;
94abbccdf2916c Eli Cohen 2020-08-04 249}
94abbccdf2916c Eli Cohen 2020-08-04 250log_entity_size = ilog2(lgcd);
94abbccdf2916c Eli Cohen 2020-08-04 251
94abbccdf2916c Eli Cohen 2020-08-04 252sglen = 1 << log_entity_size;
94abbccdf2916c Eli Cohen 2020-08-04 253nsg =
MLX5_DIV_ROUND_UP_POW2(mr->end - mr->start, log_entity_size);
94abbccdf2916c Eli Cohen 2020-08-04 254
94abbccdf2916c Eli Cohen 2020-08-04 255err =
sg_alloc_table(>sg_head, nsg, GFP_KERNEL);
94abbccdf2916c Eli Cohen 2020-08-04 256if (err)
94abbccdf2916c Eli Cohen 2020-08-04 257return err;
94abbccdf2916c Eli Cohen 2020-08-04 258
94abbccdf2916c Eli Cohen 2020-08-04 259sg = mr->sg_head.sgl;
94abbccdf2916c Eli Cohen 2020-08-04 260for (map =
vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
94abbccdf2916c Eli Cohen 2020-08-04 261 map; map =
vhost_iotlb_itree_next(map, mr->start, mr->end - 1)) {
94abbccdf2916c Eli Cohen 2020-08-04 262paend = map->addr +
maplen(map, mr);
94abbccdf2916c Eli Cohen 2020-08-04 263for (pa = map->addr; pa
< paend; pa += sglen) {
94abbccdf2916c Eli Cohen 2020-08-04 264pg =
pfn_to_page(__phys_to_pfn(pa));
94abbccdf2916c Eli Cohen 2020-08-04 265if (!sg) {
94abbccdf2916c Eli Cohen 2020-08-04 266
mlx5_vdpa_warn(mvdev, "sg null. start 0x%llx, end 0x%llx\n",
94abbccdf2916c Eli Cohen 2020-08-04 267
map->start, map->last + 1);
94abbccdf2916c Eli Cohen 2020-08-04 268err =
-ENOMEM;
94abbccdf2916c Eli Cohen 2020-08-04 269goto
err_map;
94abbccdf2916c Eli Cohen 2020-08-04 270}
94abbccdf2916c Eli Cohen 2020-08-04 271sg_set_page(sg,
pg, sglen, 0);
94abbccdf2916c Eli Cohen 2020-08-04 272sg =
sg_next(sg);
94abbccdf2916c Eli Cohen 2020-08-04 273if (!sg)
94abbccdf2916c Eli Cohen 2020-08-04 274goto
done;
94abbccdf2916c Eli Cohen 2020-08-04 275}
94abbccdf2916c Eli Cohen 2020-08-04 276}
94abbccdf2916c Eli Cohen 2020-08-04 277 done:
94abbccdf2916c Eli Cohen 2020-08-04 278mr->log_size = log_e
Re: [PATCH v2] tee: amdtee: unload TA only when its refcount becomes 0
On Mon, Apr 05, 2021 at 03:13:09PM +0530, Rijo Thomas wrote:
> @@ -340,7 +398,8 @@ int handle_open_session(struct tee_ioctl_open_session_arg
> *arg, u32 *info,
>
> int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg
> *arg)
> {
> - struct tee_cmd_load_ta cmd = {0};
> + struct tee_cmd_unload_ta unload_cmd = {0};
> + struct tee_cmd_load_ta load_cmd = {0};
It's better style to write:
struct tee_cmd_unload_ta unload_cmd = {};
It doesn't make a difference in this case, but if the first struct
member is a pointer then {0} can generate a Sparse warning. Or
depending on which bugs your version of GCC has it can affect whether
struct holes are initialized. But mostly it's just the prefered style.
regards,
dan carpenter
[kbuild] Re: [PATCH 02/10] tty/sysrq: Fix issues of code indent should use tabs
Hi Xiaofei,
url:
https://github.com/0day-ci/linux/commits/Xiaofei-Tan/tty-Fix-some-coding-style-issues/20210405-113900
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
tty-testing
config: x86_64-randconfig-m001-20210405 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/tty/sysrq.c:558 __sysrq_get_key_op() warn: curly braces intended?
vim +558 drivers/tty/sysrq.c
23cbedf812ff7c drivers/tty/sysrq.c Emil Velikov2020-05-13 549 static
const struct sysrq_key_op *__sysrq_get_key_op(int key)
bf36b9011e3c5b drivers/char/sysrq.c Andrew Morton 2006-03-25 550 {
23cbedf812ff7c drivers/tty/sysrq.c Emil Velikov2020-05-13 551 const
struct sysrq_key_op *op_p = NULL;
^1da177e4c3f41 drivers/char/sysrq.c Linus Torvalds 2005-04-16 552 int i;
^1da177e4c3f41 drivers/char/sysrq.c Linus Torvalds 2005-04-16 553
^1da177e4c3f41 drivers/char/sysrq.c Linus Torvalds 2005-04-16 554 i =
sysrq_key_table_key2index(key);
bf36b9011e3c5b drivers/char/sysrq.c Andrew Morton 2006-03-25 555 if (i
!= -1)
bf36b9011e3c5b drivers/char/sysrq.c Andrew Morton 2006-03-25 556
op_p = sysrq_key_table[i];
97f5f0cd8cd0a0 drivers/char/sysrq.c Dmitry Torokhov 2010-03-21 557
^1da177e4c3f41 drivers/char/sysrq.c Linus Torvalds 2005-04-16 @558
return op_p;
^^^
indented too far.
^1da177e4c3f41 drivers/char/sysrq.c Linus Torvalds 2005-04-16 559 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
kbuild mailing list -- kbu...@lists.01.org
To unsubscribe send an email to kbuild-le...@lists.01.org
Re: [PATCH] RDMA/addr: potential uninitialized variable in ib_nl_process_good_ip_rsep()
Could you send that and give me a reported-by? I'm going AFK for a week. regards, dan carpenter
Re: [PATCH v3 16/30] staging: rtl8723bs: tidy up some error handling in core/rtw_mlme.c
On Sat, Apr 03, 2021 at 11:50:30AM +0200, Fabio Aiuto wrote: > On Sat, Apr 03, 2021 at 11:42:47AM +0200, Fabio Aiuto wrote: > > On Sat, Apr 03, 2021 at 11:13:38AM +0200, Fabio Aiuto wrote: > > > the RT_TRACE() output is not useful so we want to delete it. In this case > > > there is no cleanup for rtw_cleanbss_cmd() required or even possible. I've > > > deleted the RT_TRACE() output and added a goto unlock to show > > > that we can't continue if rtw_createbss_cmd() fails. > > > > > > Suggested-by: David Carpenter > > > Signed-off-by: Fabio Aiuto > > > --- > > > drivers/staging/rtl8723bs/core/rtw_mlme.c | 17 +++-- > > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > Hi Dan, > > > > I put a Suggested-by tag on one patch in v3 patchset. But reading the docs > > on submitting patches I relaized later that maybe it requires your > > permission before. > > > > It' written about Reviewed-by but not about Suggested-by, should I have > > asked you before, should I? > > > > I'm sorry if I should have. > > Found it, I'm really sorry > > A Suggested-by: tag indicates that the patch idea is suggested by the person > named and ensures credit to the person for the idea. Please note that this > tag should not be added without the reporter's permission, especially if the > idea was not posted in a public forum. That said, if we diligently credit our > idea reporters, they will, hopefully, be inspired to help us again in the > future. > > I wonder if in the case of this patch was needed this tag.. Yeah, it's fine. It's all on a public list so it's not a secret. regards, dan carpenter
Re: [PATCH 01/16] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_xmit.c
On Fri, Apr 02, 2021 at 12:01:21PM +0200, Fabio Aiuto wrote:
> @@ -568,20 +561,11 @@ static s32 update_attrib_sec_info(struct adapter
> *padapter, struct pkt_attrib *p
> if (pattrib->encrypt > 0)
> memcpy(pattrib->dot118021x_UncstKey.skey,
> psta->dot118021x_UncstKey.skey, 16);
>
> - RT_TRACE(_module_rtl871x_xmit_c_, _drv_info_,
> - ("update_attrib: encrypt =%d securitypriv.sw_encrypt =%d\n",
> - pattrib->encrypt, padapter->securitypriv.sw_encrypt));
> -
> if (pattrib->encrypt &&
> - ((padapter->securitypriv.sw_encrypt == true) ||
> (psecuritypriv->hw_decrypted == false))) {
> + ((padapter->securitypriv.sw_encrypt) ||
> (!psecuritypriv->hw_decrypted)))
You've done too much clean up here. Just remove the { but leave the
== true/false comparisons.
If the patch is only changing five lines or code then fixing checkpatch
warnings on the line of code you are changing is fine, but in this case
you're doing a bunch of changes and these sort of cleanups make it hard
to review.
Ease to spot that the curly brace changed:
- ((padapter->securitypriv.sw_encrypt == true) ||
(psecuritypriv->hw_decrypted == false))) {
+ ((padapter->securitypriv.sw_encrypt == true) ||
(psecuritypriv->hw_decrypted == false)))
Hard to spot:
- ((padapter->securitypriv.sw_encrypt == true) ||
(psecuritypriv->hw_decrypted == false))) {
+ ((padapter->securitypriv.sw_encrypt) ||
(!psecuritypriv->hw_decrypted)))
regards,
dan carpenter
[PATCH] RDMA/addr: potential uninitialized variable in ib_nl_process_good_ip_rsep()
The nla_len() is less than or equal to 16. If it's less than 16 then
end of the "gid" buffer is uninitialized.
Fixes: ae43f8286730 ("IB/core: Add IP to GID netlink offload")
Signed-off-by: Dan Carpenter
---
I just spotted this in review. I think it's a bug but I'm not 100%.
drivers/infiniband/core/addr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 0abce004a959..a037ba4424bf 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -98,7 +98,7 @@ static inline bool ib_nl_is_good_ip_resp(const struct
nlmsghdr *nlh)
static void ib_nl_process_good_ip_rsep(const struct nlmsghdr *nlh)
{
const struct nlattr *head, *curr;
- union ib_gid gid;
+ union ib_gid gid = {};
struct addr_req *req;
int len, rem;
int found = 0;
--
2.30.2
Re: [PATCH 08/49] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_cmd.c
On Thu, Apr 01, 2021 at 11:51:15PM +0200, Fabio Aiuto wrote:
> On Thu, Apr 01, 2021 at 05:32:36PM +0300, Dan Carpenter wrote:
> > On Thu, Apr 01, 2021 at 03:55:37PM +0200, Fabio Aiuto wrote:
> > >
> > > Hi Dan,
> > >
> > > I have the following:
> > >
> > > if (rtw_createbss_cmd(adapter) != _SUCCESS)
> > > - RT_TRACE(_module_rtl871x_mlme_c_, _drv_err_, ("Error
> > > =>rtw_createbss_cmd status FAIL\n"));
> > > + ;
> > >
> > > will I leave
> > >
> > > if (rtw_createbss_cmd(adapter) != _SUCCESS)
> > > ;
> > >
> > > or just
> > >
> > > rtw_createbss_cmd(adapter);
> > >
> > > ?
> > >
> > > what's best from the static analysis point of view?
> > >
> > > smatch and sparse says nothing about that.
> >
> > rtw_createbss_cmd() can only fail if this allocation fails:
> >
> > pcmd = rtw_zmalloc(sizeof(struct cmd_obj));
> >
> > In current kernels, that size of small allocation will never fail. But
> > we alway write code as if every allocation can fail.
> >
> > Normally when an allocation fails then we want to return -ENOMEM and
> > clean up. But this code is an event handler for firmware events and
> > there isn't any real clean up to do. Since there is nothing we can do
> > then this is basically working and fine.
> >
> > How I would write this is:
> >
> > ret = rtw_createbss_cmd(adapter);
> > if (ret != _SUCCESS)
> > goto unlock;
> > }
> > }
> > unlock:
> > spin_unlock_bh(>lock);
> > }
> >
> > That doesn't change how the code works but it signals to the the reader
> > what your intention is. If we just remove the error handling then it's
> > ambiguous.
> >
> > rtw_createbss_cmd(adapter);
> > }
> > }
> > <-- Futurue programmer decides to add code here then figuring
> > that rtw_createbss_cmd() can fail is a problem.
> >
> > spin_unlock_bh(>lock);
> > }
> >
> > But for something like this which is maybe more subtle than just a
> > straight delete of lines of code, then consider pulling it out into its
> > own separate patch. That makes it easier to review. Put all the stuff
> > that I said in the commit message:
> >
> > ---
> > [PATCH] tidy up some error handling
> >
> > The RT_TRACE() output is not useful so we want to delete it. In this
> > case there is no cleanup for rtw_createbss_cmd() required or even
> > possible. I've deleted the RT_TRACE() output and added a goto unlock
> > to show that we can't continue if rtw_createbss_cmd() fails.
> >
> > ---
> >
> > >
> > > Checkpatch too seems to ignore it, maybe the first one is good,
> > > but I would like to be sure before sending another over 40 patches
> > > long patchset.
> >
> > Don't send 40 patches. Just send 10 at a time until you get a better
> > feel for which ones are going to get applied or not. :P It's not
> > arbitrary, and I'm definitely not trying to NAK your patches. Once you
> > learn the rules I hope that it's predictable and straight forward.
> >
> > regards,
> > dan carpenter
> >
>
> Hi Dan,
>
> sorry again. In this case:
>
> @@ -828,10 +829,11 @@ void rtw_surveydone_event_callback(struct adapter
> *adapter, u8 *pbuf)
>
> pmlmepriv->fw_state =
> WIFI_ADHOC_MASTER_STATE;
>
> - if (rtw_createbss_cmd(adapter) !=
> _SUCCESS)
> - ;
> -
> pmlmepriv->to_join = false;
> +
> + ret = rtw_createbss_cmd(adapter);
> + if (ret != _SUCCESS)
> + goto unlock;
> }
> }
>
> I decided to move the set to false of pmlepriv->to_join before
> the rtw_createbss_cmd(). In old code that statement was executed
> unconditionally and seems not to be tied to the failure of
> rtw_createbss_cmd().
>
> The eventual goto would skip this instruction so I moved it
> before.
>
> What do you think?
So when you're sending patches like this which have the potential to
change the behavior
Re: [bug report] node: Add memory-side caching attributes
On Thu, Apr 01, 2021 at 08:25:11AM -0300, Jason Gunthorpe wrote: > On Thu, Apr 01, 2021 at 12:00:39PM +0300, Dan Carpenter wrote: > > Hi Keith, > > > > I've been trying to figure out ways Smatch can check for device managed > > resources. Like adding rules that if we call dev_set_name(>bar) > > then it's device managaged and if there is a kfree(foo) without calling > > device_put(>bar) then that's a resource leak. > > It seems to be working from what I can see This check is actually more simple, and older. It just looks for device_register(dev); ... kfree(dev); I've written your proposed check of: device_register(>dev); ... kfree(foo); // warning missing device_put(>dev); But I just did that earler today and it will probably take a couple iterations to work out the kinks. Plus I'm off for a small vacation so it will be a week before I have the results from that. > > Also I wasn't able to convince myself that any locking around > node->cache_attrs exist.. > > > Of course one of the rules is that if you call device_register(dev) then > > you can't kfree(dev), it has to released with device_put(dev) and that's > > true even if the register fails. But this code here feels very > > intentional so maybe there is an exception to the rule? > > There is no exception. Open coding this: > > >282 free_name: > >283 kfree_const(dev->kobj.name); > > To avoid leaking memory from dev_set_name is a straight up layering > violation, WTF? > > node_cacheinfo_release() is just kfree(), so there is no need. > Instead (please feel free to send this Dan): Sure, I can send this (tomorrow). > > diff --git a/drivers/base/node.c b/drivers/base/node.c > index f449dbb2c74666..89c28952863977 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -319,25 +319,24 @@ void node_add_cache(unsigned int nid, struct > node_cache_attrs *cache_attrs) > return; > > dev = >dev; > + device_initialize(dev) > dev->parent = node->cache_dev; > dev->release = node_cacheinfo_release; > dev->groups = cache_groups; > if (dev_set_name(dev, "index%d", cache_attrs->level)) Is calling dev_set_name() without doing a device_initialize() a bug? I could write a check for that. regards, dan carpenter
Re: [PATCH 08/49] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_cmd.c
On Thu, Apr 01, 2021 at 03:55:37PM +0200, Fabio Aiuto wrote:
>
> Hi Dan,
>
> I have the following:
>
> if (rtw_createbss_cmd(adapter) != _SUCCESS)
> - RT_TRACE(_module_rtl871x_mlme_c_, _drv_err_, ("Error
> =>rtw_createbss_cmd status FAIL\n"));
> + ;
>
> will I leave
>
> if (rtw_createbss_cmd(adapter) != _SUCCESS)
> ;
>
> or just
>
> rtw_createbss_cmd(adapter);
>
> ?
>
> what's best from the static analysis point of view?
>
> smatch and sparse says nothing about that.
rtw_createbss_cmd() can only fail if this allocation fails:
pcmd = rtw_zmalloc(sizeof(struct cmd_obj));
In current kernels, that size of small allocation will never fail. But
we alway write code as if every allocation can fail.
Normally when an allocation fails then we want to return -ENOMEM and
clean up. But this code is an event handler for firmware events and
there isn't any real clean up to do. Since there is nothing we can do
then this is basically working and fine.
How I would write this is:
ret = rtw_createbss_cmd(adapter);
if (ret != _SUCCESS)
goto unlock;
}
}
unlock:
spin_unlock_bh(>lock);
}
That doesn't change how the code works but it signals to the the reader
what your intention is. If we just remove the error handling then it's
ambiguous.
rtw_createbss_cmd(adapter);
}
}
<-- Futurue programmer decides to add code here then figuring
that rtw_createbss_cmd() can fail is a problem.
spin_unlock_bh(>lock);
}
But for something like this which is maybe more subtle than just a
straight delete of lines of code, then consider pulling it out into its
own separate patch. That makes it easier to review. Put all the stuff
that I said in the commit message:
---
[PATCH] tidy up some error handling
The RT_TRACE() output is not useful so we want to delete it. In this
case there is no cleanup for rtw_createbss_cmd() required or even
possible. I've deleted the RT_TRACE() output and added a goto unlock
to show that we can't continue if rtw_createbss_cmd() fails.
---
>
> Checkpatch too seems to ignore it, maybe the first one is good,
> but I would like to be sure before sending another over 40 patches
> long patchset.
Don't send 40 patches. Just send 10 at a time until you get a better
feel for which ones are going to get applied or not. :P It's not
arbitrary, and I'm definitely not trying to NAK your patches. Once you
learn the rules I hope that it's predictable and straight forward.
regards,
dan carpenter
Re: [PATCH] cifsd: use kfree to free memory allocated by kzalloc
On Thu, Apr 01, 2021 at 04:39:33PM +0500, Muhammad Usama Anjum wrote:
> kfree should be used to free memory allocated by kzalloc to avoid
> any overhead and for maintaining consistency.
>
> Fixes: 5dfeb6d945 ("cifsd: use kmalloc() for small allocations")
> Signed-off-by: Muhammad Usama Anjum
> ---
> This one place was left in earlier patch. I've already received
> responsse on that patch. I'm sending a separate patch.
>
> fs/cifsd/transport_tcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifsd/transport_tcp.c b/fs/cifsd/transport_tcp.c
> index 67163efcf472..040881893417 100644
> --- a/fs/cifsd/transport_tcp.c
> +++ b/fs/cifsd/transport_tcp.c
> @@ -551,7 +551,7 @@ void ksmbd_tcp_destroy(void)
> list_for_each_entry_safe(iface, tmp, _list, entry) {
> list_del(>entry);
> kfree(iface->name);
> - ksmbd_free(iface);
> + kfree(iface);
We should just delete the ksmbd_free() function completely.
I think that cifsd is being re-written though so it might not be worth
it.
regards,
dan carpenter
Re: [PATCH -next v3] staging: greybus: camera: Switch to memdup_user_nul()
On Thu, Apr 01, 2021 at 06:36:45PM +0800, Yang Yingliang wrote: > Use memdup_user_nul() helper instead of open-coding to > simplify the code. > > Reported-by: Hulk Robot > Signed-off-by: Yang Yingliang Thanks! Reviewed-by: Dan Carpenter regards, dan carpenter
Re: [PATCH 08/49] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_cmd.c
On Thu, Apr 01, 2021 at 11:20:38AM +0200, Fabio Aiuto wrote:
> @@ -677,9 +663,8 @@ u8 rtw_createbss_cmd(struct adapter *padapter)
> u8 res = _SUCCESS;
>
> if (pmlmepriv->assoc_ssid.SsidLength == 0)
> - RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for
> Any SSid:%s\n", pmlmepriv->assoc_ssid.Ssid));
> + ;
> else
> - RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for
> SSid:%s\n", pmlmepriv->assoc_ssid.Ssid));
>
> pcmd = rtw_zmalloc(sizeof(struct cmd_obj));
This is a bug. Smatch has a check for this which hopefully would have
detected it (I haven't tested).
There are some more similar issues below as well. So generally the rule
is don't adjust the indenting if it's not related to your patch. In
some cases you have been fixing the indenting but it should be done in
a separate patch. But the other rule is that if your patch introduces
a checkpatch warning then you need to fix it in the same patch. In
this block the whole if statement should be removed. But also if you
have something like:
if (foo) {
RT_TRACE(blha blah blah);
return;
}
Then checkpatch will complain that the the curly braces are not
required. (Checkpatch might not complain for your patch but it will
complain when we re-run it with the -f option over the whole file). So
you should update this to:
if (foo)
return;
That's all considered part of deleting the RT_TRACE(). Also if there
are empty curly braces then delete those in the same patch.
I have looked over patches 1-7 and those seem basically fine. I'm not
going to review any further into this patchset because you're going to
have to redo them and I will be reviewing the v2 set later anyway. So
just look it over yourself and check for any similar issues.
regards,
dan carpenter
[bug report] node: Add memory-side caching attributes
Hi Keith,
I've been trying to figure out ways Smatch can check for device managed
resources. Like adding rules that if we call dev_set_name(>bar)
then it's device managaged and if there is a kfree(foo) without calling
device_put(>bar) then that's a resource leak.
Of course one of the rules is that if you call device_register(dev) then
you can't kfree(dev), it has to released with device_put(dev) and that's
true even if the register fails. But this code here feels very
intentional so maybe there is an exception to the rule?
The patch acc02a109b04: "node: Add memory-side caching attributes"
from Mar 11, 2019, leads to the following static checker warning:
drivers/base/node.c:285 node_init_cache_dev()
error: kfree after device_register(): 'dev'
drivers/base/node.c
263 static void node_init_cache_dev(struct node *node)
264 {
265 struct device *dev;
266
267 dev = kzalloc(sizeof(*dev), GFP_KERNEL);
268 if (!dev)
269 return;
270
271 dev->parent = >dev;
272 dev->release = node_cache_release;
273 if (dev_set_name(dev, "memory_side_cache"))
274 goto free_dev;
275
276 if (device_register(dev))
^^^
277 goto free_name;
278
279 pm_runtime_no_callbacks(dev);
280 node->cache_dev = dev;
281 return;
282 free_name:
283 kfree_const(dev->kobj.name);
284 free_dev:
285 kfree(dev);
^^^^^^
286 }
regards,
dan carpenter
Re: [PATCH -next] i2c: gpio: use DEFINE_SPINLOCK() for spinlock
On Thu, Apr 01, 2021 at 11:38:30AM +0800, chenlifu wrote: > Kindly pinging ... > > Best Regards, > Chen Lifu > > 在 2021/3/27 17:52, Chen Lifu 写道: It's to early to start asking for a response. Please wait at least two weeks. (Probably four weeks if the merge window was open). regards, dan carpenter
Re: [PATCH -next v2] staging: greybus: camera: Switch to memdup_user_nul()
On Thu, Apr 01, 2021 at 10:43:32AM +0300, Dan Carpenter wrote:
> On Thu, Apr 01, 2021 at 11:17:52AM +0800, Yang Yingliang wrote:
> > Use memdup_user_nul() helper instead of open-coding to
> > simplify the code.
> >
> > Reported-by: Hulk Robot
> > Signed-off-by: Yang Yingliang
> > ---
> > drivers/staging/greybus/camera.c | 13 +++--
> > 1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/greybus/camera.c
> > b/drivers/staging/greybus/camera.c
> > index b570e13394ac..2ecdc1bc5092 100644
> > --- a/drivers/staging/greybus/camera.c
> > +++ b/drivers/staging/greybus/camera.c
> > @@ -1120,16 +1120,9 @@ static ssize_t gb_camera_debugfs_write(struct file
> > *file,
> > if (len > 1024)
> > return -EINVAL;
> >
> > - kbuf = kmalloc(len + 1, GFP_KERNEL);
> > - if (!kbuf)
> > - return -ENOMEM;
> > -
> > - if (copy_from_user(kbuf, buf, len)) {
> > - ret = -EFAULT;
> > - goto done;
> > - }
> > -
> > - kbuf[len] = '\0';
> > + kbuf = memdup_user_nul(buf, len);
> > + if (IS_ERR(kbuf))
> > + return PTR_ERR(kbuf);;
> ^^
> There is an extra semi-colon here. Checkpatch actually catches this
> sort of typo.
So when someone makes a typo like this, my response is:
1) Let's add this to checkpatch (turns out it was already done)
2) Let's grep the kernel and fix the other instances. The command would
be something like: git grep ';;$' | grep '\.c:'
regards,
dan carpenter
Re: [PATCH -next v2] staging: greybus: camera: Switch to memdup_user_nul()
On Thu, Apr 01, 2021 at 11:17:52AM +0800, Yang Yingliang wrote:
> Use memdup_user_nul() helper instead of open-coding to
> simplify the code.
>
> Reported-by: Hulk Robot
> Signed-off-by: Yang Yingliang
> ---
> drivers/staging/greybus/camera.c | 13 +++--
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/greybus/camera.c
> b/drivers/staging/greybus/camera.c
> index b570e13394ac..2ecdc1bc5092 100644
> --- a/drivers/staging/greybus/camera.c
> +++ b/drivers/staging/greybus/camera.c
> @@ -1120,16 +1120,9 @@ static ssize_t gb_camera_debugfs_write(struct file
> *file,
> if (len > 1024)
> return -EINVAL;
>
> - kbuf = kmalloc(len + 1, GFP_KERNEL);
> - if (!kbuf)
> - return -ENOMEM;
> -
> - if (copy_from_user(kbuf, buf, len)) {
> - ret = -EFAULT;
> - goto done;
> - }
> -
> - kbuf[len] = '\0';
> + kbuf = memdup_user_nul(buf, len);
> + if (IS_ERR(kbuf))
> + return PTR_ERR(kbuf);;
^^
There is an extra semi-colon here. Checkpatch actually catches this
sort of typo.
regards,
dan carpenter
Re: [PATCH] fix NULL pointer deference crash
Hi Hassan,
url:
https://github.com/0day-ci/linux/commits/Hassan-Shahbazi/fix-NULL-pointer-deference-crash/20210401-004543
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
5e46d1b78a03d52306f21f77a4e4a144b6d31486
config: x86_64-randconfig-m001-20210330 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/video/fbdev/core/fbcon.c:1336 fbcon_cursor() warn: variable
dereferenced before check 'ops' (see line 1324)
Old smatch warnings:
drivers/video/fbdev/core/fbcon.c:3028 fbcon_get_con2fb_map_ioctl() warn:
potential spectre issue 'con2fb_map' [r]
vim +/ops +1336 drivers/video/fbdev/core/fbcon.c
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1318 static void fbcon_cursor(struct vc_data *vc, int mode)
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1319 {
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1320struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1321struct fbcon_ops *ops = info->fbcon_par;
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1322int c = scr_readw((u16 *) vc->vc_pos);
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1323
2a17d7e80f1df44 drivers/video/console/fbcon.cScot Doyle 2015-08-04
@1324ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
2a17d7e80f1df44 drivers/video/console/fbcon.cScot Doyle 2015-08-04
1325
d1e2306681ad3cb drivers/video/console/fbcon.cMichal Januszewski 2007-05-08
1326if (fbcon_is_inactive(vc, info) || vc->vc_deccm != 1)
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1327return;
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1328
c0e4b3ad67997a6 drivers/video/fbdev/core/fbcon.c Jiri Slaby 2020-06-15
1329if (vc->vc_cursor_type & CUR_SW)
acba9cd01974353 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17
1330fbcon_del_cursor_timer(info);
a5edce421848442 drivers/video/console/fbcon.cThierry Reding 2015-05-21
1331else
acba9cd01974353 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17
1332fbcon_add_cursor_timer(info);
acba9cd01974353 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17
1333
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1334ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
^
Dereferenced
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1335
1d73453653c6d4f drivers/video/fbdev/core/fbcon.c Hassan Shahbazi2021-03-31
@1336if (ops && ops->cursor)
^^^
Checked too late
06a0df4d1b8b13b drivers/video/fbdev/core/fbcon.c Linus Torvalds 2020-09-08
1337ops->cursor(vc, info, mode, get_color(vc, info, c, 1),
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1338get_color(vc, info, c, 0));
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16
1339 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH -next] staging: greybus: camera: Switch to memdup_user_nul()
On Wed, Mar 31, 2021 at 05:52:01PM +0800, Yang Yingliang wrote:
> Use memdup_user_nul() helper instead of open-coding to
> simplify the code.
>
> Reported-by: Hulk Robot
> Signed-off-by: Yang Yingliang
> ---
> drivers/staging/greybus/camera.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/greybus/camera.c
> b/drivers/staging/greybus/camera.c
> index b570e13394ac..0f005facffbc 100644
> --- a/drivers/staging/greybus/camera.c
> +++ b/drivers/staging/greybus/camera.c
> @@ -1120,16 +1120,10 @@ static ssize_t gb_camera_debugfs_write(struct file
> *file,
> if (len > 1024)
> return -EINVAL;
>
> - kbuf = kmalloc(len + 1, GFP_KERNEL);
> - if (!kbuf)
> + kbuf = memdup_user_nul(buf, len);
> + if (IS_ERR(kbuf))
> return -ENOMEM;
return PTR_ERR(kbuf);
>
> - if (copy_from_user(kbuf, buf, len)) {
> - ret = -EFAULT;
> - goto done;
> - }
> -
> - kbuf[len] = '\0';
>
Please delete this blank line so there aren't two blank lines in a row.
> ret = op->execute(gcam, kbuf, len);
regards,
dan carpenter
Re: [PATCH 01/40] staging: rtl8723bs: replace RT_TRACE with public printk wrappers in core/rtw_xmit.c
I'm sorry but we can't accept this patch.
> @@ -481,12 +481,13 @@ static s32 update_attrib_sec_info(struct adapter
> *padapter, struct pkt_attrib *p
> pattrib->mac_id = psta->mac_id;
>
> if (psta->ieee8021x_blocked == true) {
> - RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_, ("\n
> psta->ieee8021x_blocked == true\n"));
> + pr_err("%s psta->ieee8021x_blocked == true\n", DRIVER_PREFIX);
Here we have change debug code that's never printed into an error
message.
A bunch of the rest are technically "wrong" but harmless.
> if (!pxmitpriv->pallocated_frame_buf) {
> pxmitpriv->pxmit_frame_buf = NULL;
> - RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_, ("alloc xmit_frame
> fail!\n"));
> + pr_err("%s alloc xmit_frame fail!\n", DRIVER_PREFIX);
We don't print warning messages for allocation failures. Checkpatch
is supposed to complain.
I always encourage people to think about everything deeply and look at
the context and read the error messages. But if you send a patch which
mindlessly deletes all these RT_TRACE() messages, then we will apply
that.
It's unfortunate that you have to re-write the first patch in a 40
patch series. :/
regards,
dan carpenter
Re: [PATCH] media: em28xx: fix memory leak
On Wed, Mar 31, 2021 at 01:22:01PM +0500, Muhammad Usama Anjum wrote: > On Wed, 2021-03-24 at 23:07 +0500, Muhammad Usama Anjum wrote: > > If some error occurs, URB buffers should also be freed. If they aren't > > freed with the dvb here, the em28xx_dvb_fini call doesn't frees the URB > > buffers as dvb is set to NULL. The function in which error occurs should > > do all the cleanup for the allocations it had done. > > > > Tested the patch with the reproducer provided by syzbot. This patch > > fixes the memleak. > > > > Reported-by: syzbot+889397c820fa56adf...@syzkaller.appspotmail.com > > Signed-off-by: Muhammad Usama Anjum > > --- > > Resending the same path as some email addresses were missing from the > > earlier email. > > > > syzbot found the following issue on: > > > > HEAD commit:1a4431a5 Merge tag 'afs-fixes-20210315' of git://git.kerne.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=11013a7cd0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=ff6b8b2e9d5a1227 > > dashboard link: https://syzkaller.appspot.com/bug?extid=889397c820fa56adf25d > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1559ae3ad0 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=176985c6d0 > > > > drivers/media/usb/em28xx/em28xx-dvb.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c > > b/drivers/media/usb/em28xx/em28xx-dvb.c > > index 526424279637..471bd74667e3 100644 > > --- a/drivers/media/usb/em28xx/em28xx-dvb.c > > +++ b/drivers/media/usb/em28xx/em28xx-dvb.c > > @@ -2010,6 +2010,7 @@ static int em28xx_dvb_init(struct em28xx *dev) > > return result; > > > > out_free: > > + em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE); > > kfree(dvb); > > dev->dvb = NULL; > > goto ret; > > I'd received the following notice and waiting for the review: Please wait a minimum of two weeks before asking for updates. regards, dan carpenter
Re: [PATCH -next] staging: rtl8723bs: core: Remove unused variable 'res'
I've been rejecting these patches until someone updates the callers to check the return. This patch just silences the warning but the code is still totally buggy. regards, dan carpenter
Re: [PATCH] io_uring: Initialize variable before use
On Mon, Mar 22, 2021 at 11:41:58PM +0500, Muhammad Usama Anjum wrote:
> 1) Initialize the struct msghdr msg in the start of the function
> 2) Uninitialized variable msg.msg_flags can get used if branch happens to
> out_free before initialization.
>
> So initialize variable in question in the start of the function for
> simplicity in logic and use.
>
> Addresses-Coverity: ("Uninitialized variable")
> Addresses-Coverity: ("Uninitialized variable read")
This bug is a false positive.
When msg.msg_flags is uninitialized then ret is negative and min_ret is
zero.
fs/io_uring.c
4666 ret = -EINTR;
4667 out_free:
4668 if (req->flags & REQ_F_BUFFER_SELECTED)
4669 cflags = io_put_recv_kbuf(req);
4670 if (ret < min_ret || ((flags & MSG_WAITALL) && (msg.msg_flags &
(MSG_TRUNC | MSG_CTRUNC
^ ^
The first part of the condition is true so the second part is not used.
4671 req_set_fail_links(req);
4672 __io_req_complete(req, issue_flags, ret, cflags);
4673 return 0;
4674 }
regards,
dan carpenter
Re: [PATCH v2 1/2] kunit: add a KUnit test for SLUB debugging functionality
Hi,
url:
https://github.com/0day-ci/linux/commits/glittao-gmail-com/kunit-add-a-KUnit-test-for-SLUB-debugging-functionality/20210330-200635
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
1e43c377a79f9189fea8f2711b399d4e8b4e609b
config: i386-randconfig-m021-20210330 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
mm/slub.c:744 object_err() warn: should this be a bitwise op?
mm/slub.c:759 slab_err() warn: should this be a bitwise op?
mm/slub.c:807 check_bytes_and_report() warn: should this be a bitwise op?
vim +744 mm/slub.c
75c66def8d8152 Andrey Ryabinin 2015-02-13 741 void object_err(struct
kmem_cache *s, struct page *page,
81819f0fc8285a Christoph Lameter 2007-05-06 742u8
*object, char *reason)
81819f0fc8285a Christoph Lameter 2007-05-06 743 {
98f544695d3d3b Oliver Glitta 2021-03-30 @744if (!(s->flags &&
SLAB_SILENT_ERRORS)) {
^^
& was intended instead of &&
3dc5063786b273 Christoph Lameter 2008-04-23 745slab_bug(s,
"%s", reason);
2492268472e7d3 Christoph Lameter 2007-07-17 746
print_trailer(s, page, object);
81819f0fc8285a Christoph Lameter 2007-05-06 747}
98f544695d3d3b Oliver Glitta 2021-03-30 748 }
81819f0fc8285a Christoph Lameter 2007-05-06 749
a38965bf941b7c Mathieu Malaterre 2018-06-07 750 static __printf(3, 4) void
slab_err(struct kmem_cache *s, struct page *page,
d0e0ac9772f8ec Chen Gang 2013-07-15 751const
char *fmt, ...)
81819f0fc8285a Christoph Lameter 2007-05-06 752 {
81819f0fc8285a Christoph Lameter 2007-05-06 753va_list args;
81819f0fc8285a Christoph Lameter 2007-05-06 754char buf[100];
81819f0fc8285a Christoph Lameter 2007-05-06 755
2492268472e7d3 Christoph Lameter 2007-07-17 756va_start(args, fmt);
2492268472e7d3 Christoph Lameter 2007-07-17 757vsnprintf(buf,
sizeof(buf), fmt, args);
81819f0fc8285a Christoph Lameter 2007-05-06 758va_end(args);
98f544695d3d3b Oliver Glitta 2021-03-30 @759if (!(s->flags &&
SLAB_SILENT_ERRORS)) {
^
3dc5063786b273 Christoph Lameter 2008-04-23 760slab_bug(s,
"%s", buf);
2492268472e7d3 Christoph Lameter 2007-07-17 761
print_page_info(page);
81819f0fc8285a Christoph Lameter 2007-05-06 762dump_stack();
81819f0fc8285a Christoph Lameter 2007-05-06 763}
98f544695d3d3b Oliver Glitta 2021-03-30 764 }
81819f0fc8285a Christoph Lameter 2007-05-06 765
f7cb1933621bce Christoph Lameter 2010-09-29 766 static void
init_object(struct kmem_cache *s, void *object, u8 val)
81819f0fc8285a Christoph Lameter 2007-05-06 767 {
aa1ef4d7b3f67f Andrey Konovalov 2020-12-22 768u8 *p =
kasan_reset_tag(object);
81819f0fc8285a Christoph Lameter 2007-05-06 769
d86bd1bece6fc4 Joonsoo Kim 2016-03-15 770if (s->flags &
SLAB_RED_ZONE)
d86bd1bece6fc4 Joonsoo Kim 2016-03-15 771memset(p -
s->red_left_pad, val, s->red_left_pad);
d86bd1bece6fc4 Joonsoo Kim 2016-03-15 772
81819f0fc8285a Christoph Lameter 2007-05-06 773if (s->flags &
__OBJECT_POISON) {
3b0efdfa1e7193 Christoph Lameter 2012-06-13 774memset(p,
POISON_FREE, s->object_size - 1);
3b0efdfa1e7193 Christoph Lameter 2012-06-13 775
p[s->object_size - 1] = POISON_END;
81819f0fc8285a Christoph Lameter 2007-05-06 776}
81819f0fc8285a Christoph Lameter 2007-05-06 777
81819f0fc8285a Christoph Lameter 2007-05-06 778if (s->flags &
SLAB_RED_ZONE)
3b0efdfa1e7193 Christoph Lameter 2012-06-13 779memset(p +
s->object_size, val, s->inuse - s->object_size);
81819f0fc8285a Christoph Lameter 2007-05-06 780 }
81819f0fc8285a Christoph Lameter 2007-05-06 781
2492268472e7d3 Christoph Lameter 2007-07-17 782 static void
restore_bytes(struct kmem_cache *s, char *message, u8 data,
2492268472e7d3 Christoph Lameter 2007-07-17 783
void *from, void *to)
2492268472e7d3 Christoph Lameter 2007-07-17 784 {
2492268472e7d3 Christoph Lameter 2007-07-17 785slab_fix(s, "Restoring
0x%p-0x%p=0x%x\n", from, to - 1, data);
2492268472e7d3 Christoph Lameter 2007-07-17 786memset(from, data, to -
from);
2492268472e7d3 Christoph Lameter 2007-07-17 787 }
2492268472e7d3 Christoph Lameter 2007-07-17 788
2492268472e7d3 Christoph Lameter 2007-07-17 789 static int
check_bytes_and_report(struct kmem_cache *s, struct page *page,
2492268472e7d3
Re: [syzbot] WARNING in unsafe_follow_pfn
On Tue, Mar 30, 2021 at 07:04:30PM +0200, Paolo Bonzini wrote:
> On 30/03/21 17:26, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:93129492 Add linux-next specific files for 20210326
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=169ab21ad0
> > kernel config: https://syzkaller.appspot.com/x/.config?x=6f2f73285ea94c45
> > dashboard link: https://syzkaller.appspot.com/bug?extid=015dd7cdbbbc2c180c65
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=119b8d06d0
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=112e978ad0
> >
> > The issue was bisected to:
> >
> > commit d40b9fdee6dc819d8fc35f70c345cbe0394cde4c
> > Author: Daniel Vetter
> > Date: Tue Mar 16 15:33:01 2021 +
> >
> > mm: Add unsafe_follow_pfn
> >
> > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=122d2016d0
> > final oops: https://syzkaller.appspot.com/x/report.txt?x=112d2016d0
> > console output: https://syzkaller.appspot.com/x/log.txt?x=162d2016d0
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+015dd7cdbbbc2c180...@syzkaller.appspotmail.com
> > Fixes: d40b9fdee6dc ("mm: Add unsafe_follow_pfn")
>
> This is basically intentional because get_vaddr_frames is broken, isn't it?
> I think it needs to be ignored in syzkaller.
What?
The bisect is wrong (because it's blaming the commit which added the
warning instead of the commit which added the buggy caller) but the
warning is correct.
Plus users are going to be seeing this as well. According to the commit
message for 69bacee7f9ad ("mm: Add unsafe_follow_pfn") "Unfortunately
there's some users where this is not fixable (like v4l userptr of iomem
mappings)". It sort of seems crazy to dump this giant splat and then
tell users to ignore it forever because it can't be fixed... 0_0
regards,
dan carpenter
>
> Paolo
>
> > [ cut here ]
> > unsafe follow_pfn usage
> > WARNING: CPU: 1 PID: 8426 at mm/memory.c:4807 unsafe_follow_pfn+0x20f/0x260
> > mm/memory.c:4807
> > Modules linked in:
> > CPU: 0 PID: 8426 Comm: syz-executor677 Not tainted
> > 5.12.0-rc4-next-20210326-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:unsafe_follow_pfn+0x20f/0x260 mm/memory.c:4807
> > Code: 8b 7c 24 20 49 89 6d 00 e8 6e 84 64 07 e9 30 ff ff ff e8 f4 19 cb ff
> > 48 c7 c7 40 1f 76 89 c6 05 56 eb 09 0c 01 e8 34 1a 21 07 <0f> 0b e9 71 fe
> > ff ff 41 bc ea ff ff ff e9 06 ff ff ff e8 1a 65 0f
> > RSP: 0018:c9000161f660 EFLAGS: 00010282
> > RAX: RBX: 1920002c3ecc RCX:
> > RDX: 88801954d580 RSI: 815c3fd5 RDI: f520002c3ebe
> > RBP: 888023d56948 R08: R09:
> > R10: 815bd77e R11: R12: 2100
> > R13: 8880143a4010 R14: R15: 0110
> > FS: 005d1300() GS:8880b9c0() knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: 7f172c4cd6c0 CR3: 11f7 CR4: 001506f0
> > DR0: DR1: DR2:
> > DR3: DR6: fffe0ff0 DR7: 0400
> > Call Trace:
> > get_vaddr_frames+0x337/0x600
> > drivers/media/common/videobuf2/frame_vector.c:72
> > vb2_create_framevec+0x55/0xc0
> > drivers/media/common/videobuf2/videobuf2-memops.c:50
> > vb2_vmalloc_get_userptr+0xce/0x4c0
> > drivers/media/common/videobuf2/videobuf2-vmalloc.c:90
> > __prepare_userptr+0x342/0x15f0
> > drivers/media/common/videobuf2/videobuf2-core.c:1128
> > __buf_prepare+0x635/0x7d0
> > drivers/media/common/videobuf2/videobuf2-core.c:1367
> > vb2_core_qbuf+0xa9d/0x11c0
> > drivers/media/common/videobuf2/videobuf2-core.c:1658
> > vb2_qbuf+0x135/0x1a0 drivers/media/common/videobuf2/videobuf2-v4l2.c:820
> > vb2_ioctl_qbuf+0xfb/0x140
> > drivers/media/common/videobuf2/videobuf2-v4l2.c:1050
> > v4l_qbuf drivers/media/v4l2-core/v4l2-ioctl.c:2027 [inline]
> > v4l_qbuf+0x92/0xc0 drivers/media/v4l2-core/v4l2-ioctl.c:2021
> > __video_do_ioctl+0xb94/0xe20 drivers/media/v4l2-core/v4l2-ioctl.c:2951
> > video_usercopy+0x253/0x1300 drivers/media/v4l2-core/v4l2-ioctl.c:3297
> > v4l2_ioctl+0x1b3/0x250 drivers/media/v4l2-core/v4l2-dev.c:366
>
[tip:irq/core 2/2] kernel/irq/irq_sim.c:246 devm_irq_domain_create_sim() warn: passing zero to 'ERR_PTR'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
head: e6d46eded43dacf6370a7ae70f927ef4692cfcab
commit: e6d46eded43dacf6370a7ae70f927ef4692cfcab [2/2] genirq/irq_sim: Shrink
devm_irq_domain_create_sim()
config: x86_64-randconfig-m001-20210328 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
kernel/irq/irq_sim.c:246 devm_irq_domain_create_sim() warn: passing zero to
'ERR_PTR'
vim +/ERR_PTR +246 kernel/irq/irq_sim.c
337cbeb2c13eb4 Bartosz Golaszewski 2020-05-14 233 struct irq_domain
*devm_irq_domain_create_sim(struct device *dev,
337cbeb2c13eb4 Bartosz Golaszewski 2020-05-14 234
struct fwnode_handle *fwnode,
44e72c7ebf2940 Bartosz Golaszewski 2017-08-14 235
unsigned int num_irqs)
44e72c7ebf2940 Bartosz Golaszewski 2017-08-14 236 {
e6d46eded43dac Bartosz Golaszewski 2021-03-01 237 struct irq_domain
*domain;
e6d46eded43dac Bartosz Golaszewski 2021-03-01 238 int ret;
44e72c7ebf2940 Bartosz Golaszewski 2017-08-14 239
e6d46eded43dac Bartosz Golaszewski 2021-03-01 240 domain =
irq_domain_create_sim(fwnode, num_irqs);
e6d46eded43dac Bartosz Golaszewski 2021-03-01 241 if (IS_ERR(domain))
e6d46eded43dac Bartosz Golaszewski 2021-03-01 242 return domain;
44e72c7ebf2940 Bartosz Golaszewski 2017-08-14 243
e6d46eded43dac Bartosz Golaszewski 2021-03-01 244 ret =
devm_add_action_or_reset(dev, devm_irq_domain_remove_sim, domain);
e6d46eded43dac Bartosz Golaszewski 2021-03-01 245 if (!ret)
^^^
This is probably reversed. It should be "if (ret)"
e6d46eded43dac Bartosz Golaszewski 2021-03-01 @246 return
ERR_PTR(ret);
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > From: Colin Ian King
> >
> > Currently the memory pointed to by area is being freed by the
> > free_vm_area call and then area->nr_pages is referencing the
> > free'd object. Fix this swapping the order of the warn_alloc
> > message and the free.
> >
> > Addresses-Coverity: ("Read from pointer after free")
> > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > messages")
>
> i don't have this git sha. if this is -next, the sha ids aren't stable
> and shouldn't be referenced in commit logs, because these fixes should
> just be squashed into the not-yet-upstream commits.
The fixes tag won't "be referenced in the commit logs" because Andew
is going to fold it in. And the "mm/vmalloc: improve allocation failure
error messages" information is useful.
More generally there are a bunch trees which rebase often but it's
humanly impossible to remember which ones they are and there is no
automated way to track it either. If the maintainer is going to rebase
then they're going to have to think about Fixes tags a bit. Normally
rebased trees fold fixes so it's not an issue. Otherwise it would be
easy to write a script to correct the Fixes tags during a rebase.
In other words, my view is that everyone should just use the same Fixes
tag format everywhere.
regards,
dan carpenter
[kbuild] drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c:1370:4: warning: Variable 'num_of_active_display' is modified but its new value is never used. [unreadVariable]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a5e13c6df0e41702d2b2c77c8ad41677ebb065b3
commit: 75145aab7a0d865b361de687b201e8c4b76425eb drm/amdgpu/swsmu: clean up a
bunch of stale interfaces
compiler: alpha-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
In file included from drivers/gpu/drm/amd/amdgpu/../pm/swsmu/amdgpu_smu.c:
>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c:1370:4: warning: Variable
>> 'num_of_active_display' is modified but its new value is never used.
>> [unreadVariable]
num_of_active_display++;
^
vim +/num_of_active_display +1370 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1351 int smu_display_configuration_change(struct smu_context *smu,
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1352 const struct amd_pp_display_configuration
*display_config)
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1353 {
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1354int index = 0;
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1355int num_of_active_display = 0;
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1356
2b7ad277e96577 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Evan Quan 2020-05-25
1357if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
2b7ad277e96577 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Evan Quan 2020-05-25
1358return -EOPNOTSUPP;
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1359
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1360if (!display_config)
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1361return -EINVAL;
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1362
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1363mutex_lock(>mutex);
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1364
ce63d8f8b55d28 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Evan Quan 2020-06-08
1365smu_set_min_dcef_deep_sleep(smu,
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1366display_config->min_dcef_deep_sleep_set_clk
/ 100);
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1367
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1368for (index = 0; index < display_config->num_path_including_non_display;
index++) {
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1369if (display_config->displays[index].controller_id != 0)
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
@1370num_of_active_display++;
^^^
unused
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1371}
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1372
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1373mutex_unlock(>mutex);
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1374
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1375return 0;
94ed6d0cfdb867 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c Huang Rui 2019-01-11
1376 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
kbuild mailing list -- kbu...@lists.01.org
To unsubscribe send an email to kbuild-le...@lists.01.org
Re: [PATCH 2/4] rtc: abx80x: Enable SQW output
Hi Kirill,
url:
https://github.com/0day-ci/linux/commits/Kirill-Kapranov/rtc-abx80x-Enable-distributed-digital-calibration/20210329-053233
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
rtc-next
config: i386-randconfig-m021-20210328 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/rtc/rtc-abx80x.c:561 sqw_set() error: uninitialized symbol 'retval'.
vim +/retval +561 drivers/rtc/rtc-abx80x.c
3f6d456de4f347 Kirill Kapranov 2021-03-29 527 static int sqw_set(struct
i2c_client *client, const char *buf)
3f6d456de4f347 Kirill Kapranov 2021-03-29 528 {
3f6d456de4f347 Kirill Kapranov 2021-03-29 529 union abx8xx_reg_sqw
reg_sqw;
3f6d456de4f347 Kirill Kapranov 2021-03-29 530 int retval;
3f6d456de4f347 Kirill Kapranov 2021-03-29 531
3f6d456de4f347 Kirill Kapranov 2021-03-29 532 reg_sqw.val =
i2c_smbus_read_byte_data(client, ABX8XX_REG_SQW);
3f6d456de4f347 Kirill Kapranov 2021-03-29 533 if (reg_sqw.val < 0)
3f6d456de4f347 Kirill Kapranov 2021-03-29 534 goto err;
"retval" not set. Forgetting to set the error code is the canonical
bug for do nothing gotos like this.
3f6d456de4f347 Kirill Kapranov 2021-03-29 535
3f6d456de4f347 Kirill Kapranov 2021-03-29 536 if (sysfs_streq(buf,
"none")) {
3f6d456de4f347 Kirill Kapranov 2021-03-29 537 reg_sqw.sqwe =
0;
3f6d456de4f347 Kirill Kapranov 2021-03-29 538
dev_info(>dev, "sqw output disabled\n");
3f6d456de4f347 Kirill Kapranov 2021-03-29 539 } else {
3f6d456de4f347 Kirill Kapranov 2021-03-29 540 int idx =
__sysfs_match_string(sqfs, SQFS_COUNT, buf);
3f6d456de4f347 Kirill Kapranov 2021-03-29 541
3f6d456de4f347 Kirill Kapranov 2021-03-29 542 if (idx < 0)
3f6d456de4f347 Kirill Kapranov 2021-03-29 543 return
idx;
^^^
These are direct returns. Just do direct returns everywhere (more
readably, fewer bugs).
3f6d456de4f347 Kirill Kapranov 2021-03-29 544
3f6d456de4f347 Kirill Kapranov 2021-03-29 545 if
(abx80x_is_rc_mode(client) && !valid_for_rc_mode[idx])
3f6d456de4f347 Kirill Kapranov 2021-03-29 546
dev_warn(>dev, "sqw frequency %s valid only in xt mode\n",
3f6d456de4f347 Kirill Kapranov 2021-03-29 547
sqfs[idx]);
3f6d456de4f347 Kirill Kapranov 2021-03-29 548
3f6d456de4f347 Kirill Kapranov 2021-03-29 549
dev_info(>dev, "sqw output enabled @ %s\n", sqfs[idx]);
3f6d456de4f347 Kirill Kapranov 2021-03-29 550 reg_sqw.sqwe =
1;
3f6d456de4f347 Kirill Kapranov 2021-03-29 551 reg_sqw.sqws =
idx;
3f6d456de4f347 Kirill Kapranov 2021-03-29 552 }
3f6d456de4f347 Kirill Kapranov 2021-03-29 553
3f6d456de4f347 Kirill Kapranov 2021-03-29 554 retval =
i2c_smbus_write_byte_data(client, ABX8XX_REG_SQW, reg_sqw.val);
3f6d456de4f347 Kirill Kapranov 2021-03-29 555 if (retval < 0)
3f6d456de4f347 Kirill Kapranov 2021-03-29 556 goto err;
3f6d456de4f347 Kirill Kapranov 2021-03-29 557
3f6d456de4f347 Kirill Kapranov 2021-03-29 558 return 0;
3f6d456de4f347 Kirill Kapranov 2021-03-29 559 err:
3f6d456de4f347 Kirill Kapranov 2021-03-29 560 dev_err(>dev,
"Failed to set SQW\n");
3f6d456de4f347 Kirill Kapranov 2021-03-29 @561 return retval;
^
3f6d456de4f347 Kirill Kapranov 2021-03-29 562
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH 2/2] thunderbolt: Fix off by one in tb_port_find_retimer()
This array uses 1-based indexing so it corrupts memory one element
beyond of the array. Fix it by making the array one element larger.
Fixes: dacb12877d92 ("thunderbolt: Add support for on-board retimers")
Signed-off-by: Dan Carpenter
---
drivers/thunderbolt/retimer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 7a5d61604c8b..c44fad2b9fbb 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -406,7 +406,7 @@ static struct tb_retimer *tb_port_find_retimer(struct
tb_port *port, u8 index)
*/
int tb_retimer_scan(struct tb_port *port)
{
- u32 status[TB_MAX_RETIMER_INDEX] = {};
+ u32 status[TB_MAX_RETIMER_INDEX + 1] = {};
int ret, i, last_idx = 0;
if (!port->cap_usb4)
--
2.30.2
[PATCH 1/2] thunderbolt: Fix a leak in tb_retimer_add()
After the device_register() succeeds, then the correct way to clean up
is to call device_unregister(). The unregister calls both device_del()
and device_put(). Since this code was only device_del() it results in
a memory leak.
Fixes: dacb12877d92 ("thunderbolt: Add support for on-board retimers")
Signed-off-by: Dan Carpenter
---
This is from a new static checker warning. Not tested. With new
warnings it's also possible that I have misunderstood something
fundamental so review carefully etc.
drivers/thunderbolt/retimer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 620bcf586ee2..7a5d61604c8b 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -347,7 +347,7 @@ static int tb_retimer_add(struct tb_port *port, u8 index,
u32 auth_status)
ret = tb_retimer_nvm_add(rt);
if (ret) {
dev_err(>dev, "failed to add NVM devices: %d\n", ret);
- device_del(>dev);
+ device_unregister(>dev);
return ret;
}
--
2.30.2
Re: [PATCH][next] mlxsw: spectrum_router: remove redundant initialization of variable force
On Sat, Mar 27, 2021 at 10:33:34PM +, Colin King wrote:
> From: Colin Ian King
>
> The variable force is being initialized with a value that is
> never read and it is being updated later with a new value. The
> initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 6ccaa194733b..ff240e3c29f7 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5059,7 +5059,7 @@ mlxsw_sp_nexthop_obj_bucket_adj_update(struct mlxsw_sp
> *mlxsw_sp,
> {
> u16 bucket_index = info->nh_res_bucket->bucket_index;
> struct netlink_ext_ack *extack = info->extack;
> - bool force = info->nh_res_bucket->force;
> + bool force;
> char ratr_pl_new[MLXSW_REG_RATR_LEN];
> char ratr_pl[MLXSW_REG_RATR_LEN];
> u32 adj_index;
Reverse Christmas tree.
regards,
dan carpenter
Re: [PATCH] bpf: remove redundant assignment of variable id
On Fri, Mar 26, 2021 at 01:18:36PM -0700, Song Liu wrote:
> On Fri, Mar 26, 2021 at 12:45 PM Colin King wrote:
> >
> > From: Colin Ian King
> >
> > The variable id is being assigned a value that is never
> > read, the assignment is redundant and can be removed.
> >
> > Addresses-Coverity: ("Unused value")
> > Signed-off-by: Colin Ian King
>
> Acked-by: Song Liu
>
> For future patches, please prefix it as [PATCH bpf-next] for
> [PATCH bpf], based on which tree the patch should apply to.
>
You can keep asking us to do that but it's never going to happen... :P
I do this for networking but it's a massive pain in the butt and I get
it wrong 20% of the time.
regards,
dan carpenter
Re: [PATCH 05/15] staging: rtl8723bs: put parentheses on macros with complex values in include/drv_types.h
On Fri, Mar 26, 2021 at 10:09:12AM +0100, Fabio Aiuto wrote:
> fix the following checkpatch warning:
>
> ERROR: Macros with complex values should be enclosed in parentheses
> 279: FILE: drivers/staging/rtl8723bs/include/drv_types.h:279:
> +#define KEY_ARG(x) ((u8 *)(x))[0], ((u8 *)(x))[1],
> ((u8 *)(x))[2], ((u8 *)(x))[3], ((u8 *)(x))[4], ((u8 *)(x))[5], \
>
> Signed-off-by: Fabio Aiuto
> ---
> drivers/staging/rtl8723bs/include/drv_types.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/include/drv_types.h
> b/drivers/staging/rtl8723bs/include/drv_types.h
> index 1658450b386e..ead4cb9c1e5a 100644
> --- a/drivers/staging/rtl8723bs/include/drv_types.h
> +++ b/drivers/staging/rtl8723bs/include/drv_types.h
> @@ -276,9 +276,9 @@ struct cam_entry_cache {
> };
>
> #define KEY_FMT
> "%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x"
> -#define KEY_ARG(x) ((u8 *)(x))[0], ((u8 *)(x))[1], ((u8 *)(x))[2], ((u8
> *)(x))[3], ((u8 *)(x))[4], ((u8 *)(x))[5], \
> +#define KEY_ARG(x) (((u8 *)(x))[0], ((u8 *)(x))[1], ((u8 *)(x))[2], ((u8
> *)(x))[3], ((u8 *)(x))[4], ((u8 *)(x))[5], \
> ((u8 *)(x))[6], ((u8 *)(x))[7], ((u8 *)(x))[8], ((u8 *)(x))[9], ((u8
> *)(x))[10], ((u8 *)(x))[11], \
> - ((u8 *)(x))[12], ((u8 *)(x))[13], ((u8 *)(x))[14], ((u8 *)(x))[15]
> + ((u8 *)(x))[12], ((u8 *)(x))[13], ((u8 *)(x))[14], ((u8 *)(x))[15])
KEY_ARG() isn't used anywhere that I can see. Just delete it.
Please take your time when you re-write this series and think about each
change clearly and fix the underlying badness instead of just making
checkpatch happy. I would really just throw out the patchset and start
over from scratch.
regards,
dan carpenter
[PATCH] platform/surface: clean up a variable in surface_dtx_read()
The ">ddev->lock" and ">lock" are the same thing. Let's use ">lock" consistently. Signed-off-by: Dan Carpenter --- drivers/platform/surface/surface_dtx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/surface/surface_dtx.c b/drivers/platform/surface/surface_dtx.c index 1fedacf74050..63ce587e79e3 100644 --- a/drivers/platform/surface/surface_dtx.c +++ b/drivers/platform/surface/surface_dtx.c @@ -487,7 +487,7 @@ static ssize_t surface_dtx_read(struct file *file, char __user *buf, size_t coun if (status < 0) return status; - if (down_read_killable(>ddev->lock)) + if (down_read_killable(>lock)) return -ERESTARTSYS; /* Need to check that we're not shut down again. */ -- 2.30.2
