On Wed,  9 Dec 2020 18:31:09 +0200
belgin <belginsti...@hotmail.com> wrote:

Thanks a lot for the patch.

I've not tested it yet (I'll do it after sending this mail).

Beside a potential issue with CMD_LIST, everything looks good so far.

The following has potential issue:
>  enum CMD_LIST {
> -     CMD_LIST_FW_UPDATE = 0,
> -     CMD_LIST_FW_VER_BIN,
> -     CMD_LIST_FW_VER_IC,
>       CMD_LIST_CONFIG_VER,
>       CMD_LIST_GET_THRESHOLD,
>       CMD_LIST_POWER_OFF,

Before the above enum was equivalent to this one:
>  enum CMD_LIST {
>       CMD_LIST_FW_UPDATE = 0,
>       CMD_LIST_FW_VER_BIN = 1,
>       CMD_LIST_FW_VER_IC = 2,
>       CMD_LIST_CONFIG_VER = 3,
>       CMD_LIST_GET_THRESHOLD = 4,
>       CMD_LIST_POWER_OFF = 5,
>       [...]
> };

But now it's equivalent to this one:
>  enum CMD_LIST {
>       CMD_LIST_CONFIG_VER = 0,
>       CMD_LIST_GET_THRESHOLD = 1,
>       CMD_LIST_POWER_OFF = 2,
> [...]
> };

Things like that aren't usually an issue if it gets defined only once
and all its uses use the CMD_LIST_* names.

Unfortunately here, that leaks to userspace: in synaptics_sysfs.c we
have:
> SET_STORE_FN(cmd, sec_sysfs_cmd);

This probably creates /sys/devices/virtual/sec/tsp/cmd and
make the kernel call sec_sysfs_cmd when we write to that cmd file from
userspace.

Then in device/samsung/smdk4412-common/rootdir/init.smdk4x12.rc we have:
> # Permissions for touch
>     chown system radio /sys/class/sec/tsp/cmd

Do you know if this /sys/class/sec/tsp/cmd is used by something?
I've only looked rapidely in device/samsung/, and nothing seem to use
it but I wonder how we could be sure we're not breaking something here.

If only nonfree libraries are supposed to use that we could keep the
code as it as you already tested it.

Denis.

Attachment: pgpZnUdul2e9i.pgp
Description: OpenPGP digital signature

_______________________________________________
Replicant mailing list
Replicant@osuosl.org
https://lists.osuosl.org/mailman/listinfo/replicant

Reply via email to