When deleting a vma entry from a maple tree, it has to pass NULL to
vma_iter_prealloc() in order to calculate internal state of the tree,
but it passed a wrong argument. As a result, nommu kernels crashed upon
accessing a vma iterator, such as acct_collect() reading the size of
vma entries after do_munmap().
This commit fixes this issue by passing a right argument to the
preallocation call.
Fixes: b5df09226450 ("mm: set up vma iterator for vma_iter_prealloc() calls")
Cc: stable(a)vger.kernel.org
Reviewed-by: Liam R. Howlett <Liam.Howlett(a)Oracle.com>
Signed-off-by: Hajime Tazaki <thehajime(a)gmail.com>
---
mm/nommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/nommu.c b/mm/nommu.c
index 385b0c15add8..0c708f85408d 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -573,7 +573,7 @@ static int delete_vma_from_mm(struct vm_area_struct *vma)
VMA_ITERATOR(vmi, vma->vm_mm, vma->vm_start);
vma_iter_config(&vmi, vma->vm_start, vma->vm_end);
- if (vma_iter_prealloc(&vmi, vma)) {
+ if (vma_iter_prealloc(&vmi, NULL)) {
pr_warn("Allocation of vma tree for process %d failed\n",
current->pid);
return -ENOMEM;
--
2.43.0
Atomicity violation occurs during consecutive reads of
pcdev->driver_override. Consider a scenario: after pvdev->driver_override
passes the if statement, due to possible concurrency,
pvdev->driver_override may change. This leads to pvdev->driver_override
passing the condition with an old value, but entering the
return !strcmp(pcdev->driver_override, drv->name); statement with a new
value. This causes the function to return an unexpected result.
Since pvdev->driver_override is a string that is modified byte by byte,
without considering atomicity, data races may cause a partially modified
pvdev->driver_override to enter both the condition and return statements,
resulting in an error.
To fix this, we suggest protecting all reads of pvdev->driver_override
with a lock, and storing the result of the strcmp() function in a new
variable retval. This ensures that pvdev->driver_override does not change
during the entire operation, allowing the function to return the expected
result.
This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs
to extract function pairs that can be concurrently executed, and then
analyzes the instructions in the paired functions to identify possible
concurrency bugs including data races and atomicity violations.
Fixes: 5150a8f07f6c ("amba: reorder functions")
Cc: stable(a)vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
---
drivers/amba/bus.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 34bc880ca20b..e310f4f83b27 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -209,6 +209,7 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
{
struct amba_device *pcdev = to_amba_device(dev);
const struct amba_driver *pcdrv = to_amba_driver(drv);
+ int retval;
mutex_lock(&pcdev->periphid_lock);
if (!pcdev->periphid) {
@@ -230,8 +231,14 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
mutex_unlock(&pcdev->periphid_lock);
/* When driver_override is set, only bind to the matching driver */
- if (pcdev->driver_override)
- return !strcmp(pcdev->driver_override, drv->name);
+
+ device_lock(dev);
+ if (pcdev->driver_override) {
+ retval = !strcmp(pcdev->driver_override, drv->name);
+ device_unlock(dev);
+ return retval;
+ }
+ device_unlock(dev);
return amba_lookup(pcdrv->id_table, pcdev) != NULL;
}
--
2.34.1
An atomicity violation occurs during consecutive reads of the variable
cdx_dev->driver_override. Imagine a scenario: while evaluating the
statement if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override,
drv->name)), the value of cdx_dev->driver_override changes, leading to an
inconsistency where the value of cdx_dev->driver_override is the old value
when passing the non-null check, but the new value when evaluated by
strcmp(). This causes an inconsistency.
The second error occurs during the validation of cdx_dev->driver_override.
The logic of this error is similar to the first one, as the entire process
is not protected by a lock, leading to an inconsistency in the values of
cdx_dev->driver_override before and after the reads.
The third error occurs in driver_override_show() when executing the
statement return sysfs_emit(buf, "%s\n", cdx_dev->driver_override);.
Since the string changes byte by byte, it is possible for a partially
modified cdx_dev->driver_override value to be used in this statement,
leading to an incorrect return value from the program.
To fix these issues, for the first and second problems, since we need to
protect the entire process of reading the variable cdx_dev->driver_override
with a lock, we introduced a variable ret and an out block. For each branch
in this section, we replaced the return statements with assignments to the
variable ret, and then used a goto statement to directly execute the out
block, making the code overall more concise.
For the third problem, we adopted a similar approach to the one used in the
modalias_show() function, protecting the process of reading
cdx_dev->driver_override with a lock, ensuring that the program runs
correctly.
This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs to extract
function pairs that can be concurrently executed, and then analyzes the
instructions in the paired functions to identify possible concurrency bugs
including data races and atomicity violations.
Fixes: 2959ab247061 ("cdx: add the cdx bus driver")
Fixes: 48a6c7bced2a ("cdx: add device attributes")
Cc: stable(a)vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
---
drivers/cdx/cdx.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 07371cb653d3..fae03c89f818 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -268,6 +268,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
const struct cdx_driver *cdx_drv = to_cdx_driver(drv);
const struct cdx_device_id *found_id = NULL;
const struct cdx_device_id *ids;
+ int ret = false;
if (cdx_dev->is_bus)
return false;
@@ -275,28 +276,40 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
ids = cdx_drv->match_id_table;
/* When driver_override is set, only bind to the matching driver */
- if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, drv->name))
- return false;
+ device_lock(dev);
+ if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, drv->name)) {
+ ret = false;
+ goto out;
+ }
found_id = cdx_match_id(ids, cdx_dev);
- if (!found_id)
- return false;
+ if (!found_id) {
+ ret = false;
+ goto out;
+ }
do {
/*
* In case override_only was set, enforce driver_override
* matching.
*/
- if (!found_id->override_only)
- return true;
- if (cdx_dev->driver_override)
- return true;
+ if (!found_id->override_only) {
+ ret = true;
+ goto out;
+ }
+ if (cdx_dev->driver_override) {
+ ret = true;
+ goto out;
+ }
ids = found_id + 1;
found_id = cdx_match_id(ids, cdx_dev);
} while (found_id);
- return false;
+ ret = false;
+out:
+ device_unlock(dev);
+ return ret;
}
static int cdx_probe(struct device *dev)
@@ -470,8 +483,12 @@ static ssize_t driver_override_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct cdx_device *cdx_dev = to_cdx_device(dev);
+ ssize_t len;
- return sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
+ device_lock(dev);
+ len = sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
+ device_unlock(dev);
+ return len;
}
static DEVICE_ATTR_RW(driver_override);
--
2.34.1
There is a data race between the functions driver_override_show() and
driver_override_store(). In the driver_override_store() function, the
assignment to ret calls driver_set_override(), which frees the old value
while writing the new value to dev. If a race occurs, it may cause a
use-after-free (UAF) error in driver_override_show().
To fix this issue, we adopted a logic similar to the driver_override_show()
function in vmbus_drv.c, where the dev is protected by a lock to prevent
its value from changing.
This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs to extract
function pairs that can be concurrently executed, and then analyzes the
instructions in the paired functions to identify possible concurrency bugs
including data races and atomicity violations.
Fixes: 1f86a00c1159 ("bus/fsl-mc: add support for 'driver_override' in the mc-bus")
Cc: stable(a)vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 930d8a3ba722..62a9da88b4c9 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -201,8 +201,12 @@ static ssize_t driver_override_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ ssize_t len;
- return snprintf(buf, PAGE_SIZE, "%s\n", mc_dev->driver_override);
+ device_lock(dev);
+ len = snprintf(buf, PAGE_SIZE, "%s\n", mc_dev->driver_override);
+ device_unlock(dev);
+ return len;
}
static DEVICE_ATTR_RW(driver_override);
--
2.34.1
On Tue, Nov 12, 2024 at 3:02 PM Len Brown <lenb(a)kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 8:14 AM Rafael J. Wysocki <rafael(a)kernel.org> wrote:
> >
> > On Tue, Nov 12, 2024 at 2:12 PM Len Brown <lenb(a)kernel.org> wrote:
> > >
> > > On Tue, Nov 12, 2024 at 6:44 AM Rafael J. Wysocki <rafael(a)kernel.org> wrote:
> > >
> > > > > - if (boot_cpu_has(X86_FEATURE_MWAIT) && c->x86_vfm == INTEL_ATOM_GOLDMONT)
> > > > > + if (boot_cpu_has(X86_FEATURE_MWAIT) &&
> > > > > + (c->x86_vfm == INTEL_ATOM_GOLDMONT
> > > > > + || c->x86_vfm == INTEL_LUNARLAKE_M))
> > > >
> > > > I would put the || at the end of the previous line, that is
> > >
> > >
> > > It isn't my personal preference for human readability either,
> > > but this is what scripts/Lindent does...
> >
> > Well, it doesn't match the coding style of the first line ...
>
> Fair observation.
>
> I'll bite.
>
> If you took the existing intel.c and added it as a patch to the kernel,
> the resulting checkpatch would have 6 errors and 33 warnings.
>
> If you ran Lindent on the existing intel.c, the resulting diff would be
> 408 lines -- 1 file changed, 232 insertions(+), 176 deletions(-)
>
> This for a file that is only 1300 lines long.
>
> If whitespace nirvana is the goal, tools are the answer, not the valuable
> cycles of human reviewers.
Well, the advice always given is to follow the coding style of the
given fine in the first place.
checkpatch reflects the preferences of its author is this particular
respect and maintainers' preferences tend to differ from one to
another.
From: Parth Pancholi <parth.pancholi(a)toradex.com>
Replace lz4c with lz4 for kernel image compression.
Although lz4 and lz4c are functionally similar, lz4c has been
deprecated upstream since 2018. Since as early as Ubuntu 16.04 and
Fedora 25, lz4 and lz4c have been packaged together, making it safe
to update the requirement from lz4c to lz4. Consequently, some
distributions and build systems, such as OpenEmbedded, have fully
transitioned to using lz4. OpenEmbedded core adopted this change in
commit fe167e082cbd ("bitbake.conf: require lz4 instead of lz4c"),
causing compatibility issues when building the mainline kernel in
the latest OpenEmbedded environment, as seen in the errors below.
This change maintains compatibility with current kernel builds
because both tools have a similar command-line interface while
fixing the mainline kernel build failures with the latest master
OpenEmbedded builds associated with the mentioned compatibility
issues.
LZ4 arch/arm/boot/compressed/piggy_data
/bin/sh: 1: lz4c: not found
...
...
ERROR: oe_runmake failed
Cc: stable(a)vger.kernel.org
Link: https://github.com/lz4/lz4/pull/553
Suggested-by: Francesco Dolcini <francesco.dolcini(a)toradex.com>
Signed-off-by: Parth Pancholi <parth.pancholi(a)toradex.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 79192a3024bf..7630f763f5b2 100644
--- a/Makefile
+++ b/Makefile
@@ -508,7 +508,7 @@ KGZIP = gzip
KBZIP2 = bzip2
KLZOP = lzop
LZMA = lzma
-LZ4 = lz4c
+LZ4 = lz4
XZ = xz
ZSTD = zstd
--
2.34.1
On Tue, Nov 12, 2024 at 2:12 PM Len Brown <lenb(a)kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 6:44 AM Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>
> > > - if (boot_cpu_has(X86_FEATURE_MWAIT) && c->x86_vfm == INTEL_ATOM_GOLDMONT)
> > > + if (boot_cpu_has(X86_FEATURE_MWAIT) &&
> > > + (c->x86_vfm == INTEL_ATOM_GOLDMONT
> > > + || c->x86_vfm == INTEL_LUNARLAKE_M))
> >
> > I would put the || at the end of the previous line, that is
>
>
> It isn't my personal preference for human readability either,
> but this is what scripts/Lindent does...
Well, it doesn't match the coding style of the first line ...
This series primarily adds check at relevant places in venus driver where there
are possible OOB accesses due to unexpected payload from venus firmware. The
patches describes the specific OOB possibility.
Please review and share your feedback.
Signed-off-by: Vikash Garodia <quic_vgarodia(a)quicinc.com>
---
Vikash Garodia (4):
media: venus: hfi_parser: add check to avoid out of bound access
media: venus: hfi_parser: avoid OOB access beyond payload word count
media: venus: hfi: add check to handle incorrect queue size
media: venus: hfi: add a check to handle OOB in sfr region
drivers/media/platform/qcom/venus/hfi_parser.c | 6 +++++-
drivers/media/platform/qcom/venus/hfi_venus.c | 15 +++++++++++++--
2 files changed, 18 insertions(+), 3 deletions(-)
---
base-commit: c7ccf3683ac9746b263b0502255f5ce47f64fe0a
change-id: 20241104-venus_oob-0343b143d61d
Best regards,
--
Vikash Garodia <quic_vgarodia(a)quicinc.com>