During SCM probe, to identify the SCM convention, scm call is made with
SMC_CONVENTION_ARM_64 followed by SMC_CONVENTION_ARM_32. Based on the
result what convention to be used is decided.
IPQ chipsets starting from IPQ807x, supports both 32bit and 64bit kernel
variants, however TZ firmware runs in 64bit mode. When running on 32bit
kernel, scm call is made with SMC_CONVENTION_ARM_64 is causing the
system crash, due to the difference in the register sets between ARM and
AARCH64, which is …
[View More]accessed by the TZ.
To avoid this, use SMC_CONVENTION_ARM_64 only on ARM64 builds.
Cc: stable(a)vger.kernel.org
Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions")
Signed-off-by: Kathiravan T <quic_kathirav(a)quicinc.com>
---
Changes in V2:
- Added the Fixes tag and cc'd stable mailing list
drivers/firmware/qcom_scm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index fde33acd46b7..db6754db48a0 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -171,6 +171,7 @@ static enum qcom_scm_convention __get_convention(void)
if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN))
return qcom_scm_convention;
+#if IS_ENABLED(CONFIG_ARM64)
/*
* Device isn't required as there is only one argument - no device
* needed to dma_map_single to secure world
@@ -191,6 +192,7 @@ static enum qcom_scm_convention __get_convention(void)
forced = true;
goto found;
}
+#endif
probed_convention = SMC_CONVENTION_ARM_32;
ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true);
--
2.17.1
[View Less]
The NAND core complies with the ONFI specification, which itself
mentions that after any program or erase operation, a status check
should be performed to see whether the operation was finished *and*
successful.
The NAND core offers helpers to finish a page write (sending the
"PAGE PROG" command, waiting for the NAND chip to be ready again, and
checking the operation status). But in some cases, advanced controller
drivers might want to optimize this and craft their own page write
helper to …
[View More]leverage additional hardware capabilities, thus not always
using the core facilities.
Some drivers, like this one, do not use the core helper to finish a page
write because the final cycles are automatically managed by the
hardware. In this case, the additional care must be taken to manually
perform the final status check.
Let's read the NAND chip status at the end of the page write helper and
return -EIO upon error.
Cc: stable(a)vger.kernel.org
Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
Reported-by: Aviram Dali <aviramd(a)marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal(a)bootlin.com>
---
Hello Aviram,
I have not tested this, but based on your report I believe the status
check is indeed missing here and could sometimes lead to unnoticed
partial writes.
Please test on your side and reply with your Tested-by if you validate
the change.
Any backport on kernels predating v4.17 will likely fail because of a
folder rename, so you will have to do the backport manually if needed.
Thanks,
Miquèl
---
drivers/mtd/nand/raw/marvell_nand.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 30c15e4e1cc0..576441095012 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -1162,6 +1162,7 @@ static int marvell_nfc_hw_ecc_hmg_do_write_page(struct nand_chip *chip,
.ndcb[2] = NDCB2_ADDR5_PAGE(page),
};
unsigned int oob_bytes = lt->spare_bytes + (raw ? lt->ecc_bytes : 0);
+ u8 status;
int ret;
/* NFCv2 needs more information about the operation being executed */
@@ -1195,7 +1196,18 @@ static int marvell_nfc_hw_ecc_hmg_do_write_page(struct nand_chip *chip,
ret = marvell_nfc_wait_op(chip,
PSEC_TO_MSEC(sdr->tPROG_max));
- return ret;
+ if (ret)
+ return ret;
+
+ /* Check write status on the chip side */
+ ret = nand_status_op(chip, &status);
+ if (ret)
+ return ret;
+
+ if (status & NAND_STATUS_FAIL)
+ return -EIO;
+
+ return 0;
}
static int marvell_nfc_hw_ecc_hmg_write_page_raw(struct nand_chip *chip,
@@ -1624,6 +1636,7 @@ static int marvell_nfc_hw_ecc_bch_write_page(struct nand_chip *chip,
int data_len = lt->data_bytes;
int spare_len = lt->spare_bytes;
int chunk, ret;
+ u8 status;
marvell_nfc_select_target(chip, chip->cur_cs);
@@ -1660,6 +1673,14 @@ static int marvell_nfc_hw_ecc_bch_write_page(struct nand_chip *chip,
if (ret)
return ret;
+ /* Check write status on the chip side */
+ ret = nand_status_op(chip, &status);
+ if (ret)
+ return ret;
+
+ if (status & NAND_STATUS_FAIL)
+ return -EIO;
+
return 0;
}
--
2.34.1
[View Less]
Similar to the rk817 codec alias that was missing, the rk817 charger
driver is missing a module alias as well. This absence prevents the
driver from autoprobing on OF systems when it is built as a module.
Add the right MODULE_ALIAS to fix this.
Fixes: 11cb8da0189b ("power: supply: Add charger driver for Rockchip RK817")
Cc: stable(a)vger.kernel.org
Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas(a)gmail.com>
---
drivers/power/supply/rk817_charger.c | 1 +
1 file changed, 1 …
[View More]insertion(+)
diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c
index 1a2143641e66..76b991e112da 100644
--- a/drivers/power/supply/rk817_charger.c
+++ b/drivers/power/supply/rk817_charger.c
@@ -1211,3 +1211,4 @@ MODULE_DESCRIPTION("Battery power supply driver for RK817 PMIC");
MODULE_AUTHOR("Maya Matuszczyk <maccraft123mc(a)gmail.com>");
MODULE_AUTHOR("Chris Morgan <macromorgan(a)hotmail.com>");
MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:rk817-charger");
--
2.41.0
[View Less]
When introducing support for processed channels I needed
to invert the expression:
if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
!iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE))
dev_err(dev, "source channel does not support raw/scale\n");
To the inverse, meaning detect when we can usse raw+scale
rather than when we can not. This was the result:
if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE))
…
[View More]dev_info(dev, "using raw+scale source channel\n");
Ooops. Spot the error. Yep old George Boole came up and bit me.
That should be an &&.
The current code "mostly works" because we have not run into
systems supporting only raw but not scale or only scale but not
raw, and I doubt there are few using the rescaler on anything
such, but let's fix the logic.
Cc: Liam Beguin <liambeguin(a)gmail.com>
Cc: stable(a)vger.kernel.org
Fixes: 53ebee949980 ("iio: afe: iio-rescale: Support processed channels")
Signed-off-by: Linus Walleij <linus.walleij(a)linaro.org>
---
drivers/iio/afe/iio-rescale.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
index 7e511293d6d1..dc426e1484f0 100644
--- a/drivers/iio/afe/iio-rescale.c
+++ b/drivers/iio/afe/iio-rescale.c
@@ -278,7 +278,7 @@ static int rescale_configure_channel(struct device *dev,
chan->ext_info = rescale->ext_info;
chan->type = rescale->cfg->type;
- if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
+ if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) &&
iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
dev_info(dev, "using raw+scale source channel\n");
} else if (iio_channel_has_info(schan, IIO_CHAN_INFO_PROCESSED)) {
--
2.35.3
[View Less]
By abusing new methods of file descriptor transferal it is possible to
break the assumption that f_pos_lock doesn't need to be acquired when
the caller is single threaded and the file_count(file) is one aka when
the caller holds the only reference to the file.
The explanation has two parts. The first part is about a new io_uring
interface that isn't merged yet where I realized that only acquiring the
lock when multiple references to a file are held would be broken in some
circumstances. The …
[View More]second part illustrates that this assumption is
already broken by abusing another interface.
During review of the io_uring patchset for adding getdents support I
pointed out various problems in the locking assumptions that were made.
Effectively, the guts of __fdget_pos() were copied out and a trylock
scheme for f_pos_lock was introduced.
The approach overall is workable but the locking was wrong because it
copied the file_count(file) greater than one optimization that
__fdget_pos() had into io_uring. But this assumption is broken when
fixed file descriptors are used.
Short reminder of fixed files here via some C pseudo code:
T1 (still single threaded)
fd_register = open("/some/file") -----------------> f_count == 1
fd_fixed = io_uring_register_file(fd_register)
-> io_sqe_files_register(fd_register)
-> fget(fd_register) --------------------------> f_count == 2
-> io_fixed_file_set(fd_register)
close(fd_register);
-> close_fd(fd_register)
-> file = pick_file()
-> filp_close(file)
-> fput(file) ------------------------------> f_count == 1
The caller has now traded a regular file descriptor reference for a
fixed file reference. Such fixed files never use fget() again and thus
fully eliminate any fget()/fput() overhead.
However, for getdents f_pos_lock needs to be acquired as state may often
be kept that requires synchronization with seek and other concurrent
getdent requests.
Since io_uring is an asynchronous interface it is of course possible to
register multiple concurrent getdent calls that aren't synchronized by
io_uring as that's never done unless the user requests specific
chaining. And since the reference count for fixed files is always one
it's possible to create multiple racing getdent requests if locking is
conditional on file_count(file) being greater than one.
That wouldn't be a problem since io_uring can just unconditionally take
f_pos_lock and eliminate this problem for fixed files since they aren't
usable with the regular system call api.
However, while thinking about this I realized that a while ago the
file_count(file) greater than one optimization was already broken and
that concurrent read/write/getdents/seek calls are possible in the
regular system call api.
The pidfd_getfd() system call allows a caller with ptrace_may_access()
abilities on another process to steal a file descriptor from this
process. This system call is used by debuggers, container runtimes,
system call supervisors, networking proxies etc (cf. [1]-[3]). So while
it is a special interest system call it is used in common tools.
Via pidfd_getfd() it's possible to intercept syscalls such as
connect(2). For example, a container manager might want to rewrite the
connect(2) request to something other than the task intended for
security reasons or because the task lacks the necessary information
about the networking layout to connect to the right endpoint. In these
cases pidfd_getfd(2) can be used to retrieve a copy of the file
descriptor of the task and perform the connect(2) for it.
When used in combination with the seccomp notifier things are pretty
simple. The intercepted task itself is blocked in the seccomp code. That
code runs before the actual requested system call is performed. So the
task is blocked before any system call is performed. The seccomp
notifier will then notify the supervising process which holds the
seccomp notifier fd.
The supervisor can now call pidfd_getfd() while the target process is
blocked and steal the file descriptor. It can then perform whatever
operation it wants on that file descriptor and then tell seccomp to
either return a specific return value to the task once it is unblocked
or even continue the task using the SECCOMP_USER_NOTIF_FLAG_CONTINUE
flag.
One of the most useful things for pidfd_getfd() is that the target task
from which the caller is about to steal a file descriptor doesn't need
to be blocked or stopped. But that makes it possible for the target
process to be in the middle of a read/write/getdents/seek system call
while the caller is stealing the file on which that
read/write/getdents/seek was issued and issuing a concurrent
read/write/getdents/seek system call:
P1 P2
getdents(fd)
-> fdget_pos()
{
* @file->f_count == 1
* @current->files->count == 1
=> not taking @file->f_pos_lock
}
fd = pidfd_getfd(fd)
-> __pidfd_fget()
-> fget_task() ----> f_count = 2
-> receive_fd()
-> get_file() --> f_count = 3
-> fd_install()
-> fput() ---------> f_count = 2
getdents(fd)
-> fdget_pos()
{
* @file->f_count == 1
* @file->f_count == 2
=> taking @file->f_pos_lock and bumping @file->f_count = 3
}
-> vfs_readdir() -> vfs_readdir()
-> fdput() -> fdput()---------> f_count = 2
Although I'm not happy about it, I'm somewhat confident that this
analysis is correct but that the race is hard to hit in real life but
with some ingenuity it might be possible to make it more reliable.
However, I was lazy and introduced a luscious 10 second delay after
fdget_pos() succeeded into vfs_read() that was triggerable by passing -1
as the buffer size to the read() system call. I then used the very very
ugly program in [4] to enforce the order illustrated above. Lastly, I
wrote a bpftrace program which attached a kretfunc to __fdget_pos() and
a kfunc and a kretfunc to vfs_read(). The bpftrace program would only
trigger if the target file with a specific inode number was read:
Attaching 3 probes...
fdget_pos() on file pointer 0xffff8b3500d6a300: name(stealme) | i_ino(1591) | pid(9690) FDPUT_POS_UNLOCK(0), count(1) => mutex not acquired
vfs_read() on file pointer 0xffff8b3500d6a300: pid 9690 started reading from name(stealme) | i_ino(1591)
fdget_pos() on file pointer 0xffff8b3500d6a300: name(stealme) | i_ino(1591) | pid(9691) FDPUT_POS_UNLOCK(2), count(2) => mutex acquired
vfs_read() on file pointer 0xffff8b3500d6a300: pid 9691 started reading from name(stealme) | i_ino(1591)
vfs_read() on file pointer 0xffff8b3500d6a300: pid 9690 finished reading from name(stealme) | i_ino(1591)
vfs_read() on file pointer 0xffff8b3500d6a300: pid 9691 finished reading from name(stealme) | i_ino(1591)
I thought about various ways to fix this such as introducing some
mechanism to refuse pidfd_getfd() if the target process is in the middle
of an fdget_pos() protected system call with file count being one up to
just not caring about it at all. But it is pretty nasty to leave this
race open and it feels like something that'll bite us later anyway.
Plus, with io_uring having to acquire f_pos_lock unconditionally anyway
for getdents support I'm not sure that deviation in locking semantics
here is worth keeping. So remove the locking optimization for file count
being one. We'll see if this has a significant performance impact.
This survives xfstests and LTP.
Link: https://github.com/rr-debugger/rr [1]
Link: https://github.com/opencontainers/runc [2]
Link: https://brauner.io/2020/07/23/seccomp-notify.html [3]
Link: https://gist.github.com/brauner/599d31048c8adcb3aa0c537b76c89e12 [4]
Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall")
Cc: stable(a)vger.kernel.org
Signed-off-by: Christian Brauner <brauner(a)kernel.org>
---
---
fs/file.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/file.c b/fs/file.c
index 7893ea161d77..35c62b54c9d6 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -1042,10 +1042,8 @@ unsigned long __fdget_pos(unsigned int fd)
struct file *file = (struct file *)(v & ~3);
if (file && (file->f_mode & FMODE_ATOMIC_POS)) {
- if (file_count(file) > 1) {
- v |= FDPUT_POS_UNLOCK;
- mutex_lock(&file->f_pos_lock);
- }
+ v |= FDPUT_POS_UNLOCK;
+ mutex_lock(&file->f_pos_lock);
}
return v;
}
---
base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef
change-id: 20230724-vfs-fdget_pos-c6540e3ed7d9
[View Less]
Hi,
I notice a regression report on Bugzilla [1]. Quoting from it:
> vmlinuz-6.3.7-200.fc38.x86_64 and vmlinuz-6.3.8-200.fc38.x86_64
>
> in the switch tv channel to another vlc stay blocked and and the loading of firmware newer ending. power must be switch to off.
> from dmesg:
>
> dvb-tuner-si2141-a10-01.fw
> dvb-demod-si2168-d60-01.fw
> firmware version: D 6.0.13
>
> restart of dvb modules is impossible, They stay working.
>
> kernel 6.3.5-200.fc38.…
[View More]x86_64 works OK. I don't how works kernel 6.3.6-200.fc38.x86_64
>
> this same error is in vmlinuz-6.3.5-200.fc38.x86_64 and earliers but there is the occurence accesptable.
>
> listing from 6.3.5 kernel:
>
> [ 3378.026035] si2168 7-0064: firmware version: D 6.0.13
> [ 3431.891579] si2168 7-0064: downloading firmware from file 'dvb-demod-si2168-d60-01.fw'
> [ 3434.037278] usb 1-4: dvb_usb_v2: 2nd usb_bulk_msg() failed=-110
> [ 3434.037289] si2168 7-0064: firmware download failed -110
> [ 3436.085265] usb 1-4: dvb_usb_v2: usb_bulk_msg() failed=-110
>
> lsmod |grep dvb
> dvb_usb_dvbsky 32768 2
> dvb_usb_v2 57344 1 dvb_usb_dvbsky
> m88ds3103 49152 1 dvb_usb_dvbsky
> dvb_core 192512 3 dvb_usb_v2,m88ds3103,dvb_usb_dvbsky
> m
>
> modprobe -r dvb_usb_dvbsky
> modprobe: FATAL: Module dvb_usb_dvbsky is in use.
>
> ?????? The problem in dvb_usb_dvbsky or firmware ?????
>
> The answer from https://bugzilla.rpmfusion.org was:
>
> If it was working with previous kernel this is likely a kernel regression.
> please forward to the dvb sub-system maintainers.
>
> this problen insist a long time (years), but not too horible
See Bugzilla for the full thread and attached dmesg and lsusb.
Thorsten: The reporter can't perform bisection for he has problem
compiling his own vanilla kernel (see comment 8 on the Bugzilla
ticket, albeit with translated make(1) output). Can you take a look on
it?
Anyway, I'm adding it to regzbot (as stable-specific regression
for now):
#regzbot introduced: v6.3.5..v6.3.7 https://bugzilla.kernel.org/show_bug.cgi?id=217566
#regzbot title: switching TV channel causes VLC and firmware loading hang
Thanks.
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=217566
--
An old man doll... just what I always wanted! - Clara
[View Less]