September 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] Playback volumes
by GitHub pull_request - opened 27 Sep '19

27 Sep '19

alsa-project/alsa-ucm-conf pull request #1 was opened from juimonen: Request URL : https://github.com/alsa-project/alsa-ucm-conf/pull/1 Patch URL : https://github.com/alsa-project/alsa-ucm-conf/pull/1.patch Repository URL: https://github.com/alsa-project/alsa-ucm-conf

1 0

[alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support
by Brent Lu 27 Sep '19

27 Sep '19

BDW boards using this machine driver supports only stereo capture and playback. Implement a constraint to enforce it. Signed-off-by: Brent Lu <brent.lu(a)intel.com> --- sound/soc/intel/boards/bdw-rt5677.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index 4a4d335..a312b55 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -22,6 +22,8 @@ #include "../../codecs/rt5677.h" +#define DUAL_CHANNEL 2 + struct bdw_rt5677_priv { struct gpio_desc *gpio_hp_en; struct snd_soc_component *component; @@ -245,6 +247,36 @@ static int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd) return 0; } +static const unsigned int channels[] = { + DUAL_CHANNEL, +}; + +static const struct snd_pcm_hw_constraint_list constraints_channels = { + .count = ARRAY_SIZE(channels), + .list = channels, + .mask = 0, +}; + +static int bdw_fe_startup(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + + /* + * On this platform for PCM device we support, + * stereo + */ + + runtime->hw.channels_max = DUAL_CHANNEL; + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, + &constraints_channels); + + return 0; +} + +static const struct snd_soc_ops bdw_rt5677_fe_ops = { + .startup = bdw_fe_startup, +}; + /* broadwell digital audio interface glue - connects codec <--> CPU */ SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY())); @@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { }, .dpcm_capture = 1, .dpcm_playback = 1, + .ops = &bdw_rt5677_fe_ops, SND_SOC_DAILINK_REG(fe, dummy, platform), }, -- 2.7.4

4 9

[alsa-devel] [PATCH 0/4] ASoC: mt8183: fix audio playback slowly after playback
by Jiaxin Yu 27 Sep '19

27 Sep '19

This series patches add reset controller for MT8183, and audio will use it in machine driver during bootup, they depend on the following patch: 1. this series add support reset-controller in infra [v5,2/2] clk: reset: Modify reset-controller driver *** BLURB HERE *** Jiaxin Yu (2): WIP: dt-bindings: medaitek: mt8183: add property "resets" && "reset-names" WIP: ASoC: mt8183: fix audio playback slowly after playback during bootup yong.liang (2): WIP: dt-bindings: mediatek: mt8183: Add #reset-cells WIP: watchdog: mtk_wdt: mt8183: Add reset controller .../bindings/sound/mt8183-afe-pcm.txt | 6 + .../devicetree/bindings/watchdog/mtk-wdt.txt | 9 +- drivers/watchdog/Kconfig | 1 + drivers/watchdog/mtk_wdt.c | 110 +++++++++++++++++- .../reset-controller/mt8183-resets.h | 13 +++ sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 15 +++ 6 files changed, 150 insertions(+), 4 deletions(-) -- 2.18.0

1 4

[alsa-devel] [PATCH] ALSA: hda - Force polling mode on CML for fixing codec communication
by Sathyanarayana Nujella 26 Sep '19

26 Sep '19

We observed the same issue as reported by commit a8d7bde23e7130686b7662 ("ALSA: hda - Force polling mode on CFL for fixing codec communication") Apply the same fix to CML. Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella(a)intel.com> Signed-off-by: Joyce Toh <joyce.toh(a)intel.com> --- sound/pci/hda/hda_intel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 91e71be42fa4..e6997a5bf098 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -370,6 +370,7 @@ enum { #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98) #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348) #define IS_CNL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9dc8) +#define IS_CML(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x02c8) static char *driver_short_names[] = { [AZX_DRIVER_ICH] = "HDA Intel", @@ -1754,7 +1755,7 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci, azx_bus(chip)->dma_type = SNDRV_DMA_TYPE_DEV_UC; /* Workaround for a communication error on CFL (bko#199007) and CNL */ - if (IS_CFL(pci) || IS_CNL(pci)) + if (IS_CFL(pci) || IS_CNL(pci) || IS_CML(pci)) azx_bus(chip)->polling_mode = 1; if (chip->driver_type == AZX_DRIVER_NVIDIA) { -- 2.7.4

4 3

[alsa-devel] [PATCH] ASoC: topology: Fix a signedness bug in soc_tplg_dapm_widget_create()
by Dan Carpenter 26 Sep '19

26 Sep '19

The "template.id" variable is an enum and in this context GCC will treat it as an unsigned int so it can never be less than zero. Fixes: 8a9782346dcc ("ASoC: topology: Add topology core") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- sound/soc/soc-topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index aa9a1fca46fa..0fd032914a31 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1582,7 +1582,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, /* map user to kernel widget ID */ template.id = get_widget_id(le32_to_cpu(w->id)); - if (template.id < 0) + if ((int)template.id < 0) return template.id; /* strings are allocated here, but used and freed by the widget */ -- 2.20.1

2 1

[alsa-devel] [PATCH] ASoC: amd: Missing Initialization of IRQFLAGS
by Ravulapati Vishnu vardhan rao 26 Sep '19

26 Sep '19

Fix for missing initialization of IRQFLAGS in ACP-PCI driver and Missing Macro of ACP3x_DEVS. Follow up to IDb33df346 Signed-off-by: Ravulapati Vishnu vardhan rao <Vishnuvardhanrao.Ravulapati(a)amd.com> --- sound/soc/amd/raven/pci-acp3x.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/amd/raven/pci-acp3x.c b/sound/soc/amd/raven/pci-acp3x.c index 627798a..3887ea0 100644 --- a/sound/soc/amd/raven/pci-acp3x.c +++ b/sound/soc/amd/raven/pci-acp3x.c @@ -219,7 +219,7 @@ static int snd_acp3x_probe(struct pci_dev *pci, sizeof(struct resource) * 4, GFP_KERNEL); adata->cell = devm_kzalloc(&pci->dev, - sizeof(struct mfd_cell) * 3, + sizeof(struct mfd_cell) * ACP3x_DEVS, GFP_KERNEL); if (!adata->cell) { ret = -ENOMEM; @@ -260,6 +260,7 @@ static int snd_acp3x_probe(struct pci_dev *pci, adata->res[3].flags = IORESOURCE_IRQ; adata->res[3].start = pci->irq; adata->res[3].end = adata->res[3].start; + irqflags = 0; adata->acp3x_audio_mode = ACP3x_I2S_MODE; @@ -282,8 +283,9 @@ static int snd_acp3x_probe(struct pci_dev *pci, adata->cell[2].platform_data = &i2s_pdata[1]; adata->cell[2].pdata_size = sizeof(struct i2s_platform_data); - r = mfd_add_hotplug_devices(adata->parent, adata->cell, 3); - for (i = 0; i < 3 ; i++) + r = mfd_add_hotplug_devices(adata->parent, adata->cell, + ACP3x_DEVS); + for (i = 0; i < ACP3x_DEVS ; i++) dev = get_mfd_cell_dev(adata->cell[i].name, i); break; -- 2.7.4

2 1

[alsa-devel] [PATCH] dt-bindings: sound: Convert Samsung Exynos Odroid XU3/XU4 audio complex to dt-schema
by Marek Szyprowski 26 Sep '19

26 Sep '19

From: Maciej Falkowski <m.falkowski(a)samsung.com> Convert Samsung Exynos Odroid XU3/XU4 audio complex with MAX98090 codec to newer dt-schema format. 'clocks' property is unneeded in the bindings and is left undefined in 'properties'. 'samsung,audio-widgets' and 'samsung,audio-routing' are optional from driver perspective and they are set as unrequired. Signed-off-by: Maciej Falkowski <m.falkowski(a)samsung.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> --- .../bindings/sound/samsung,odroid.txt | 54 ----------- .../bindings/sound/samsung,odroid.yaml | 91 +++++++++++++++++++ 2 files changed, 91 insertions(+), 54 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/samsung,odroid.txt create mode 100644 Documentation/devicetree/bindings/sound/samsung,odroid.yaml diff --git a/Documentation/devicetree/bindings/sound/samsung,odroid.txt b/Documentation/devicetree/bindings/sound/samsung,odroid.txt deleted file mode 100644 index e9da2200e173..000000000000 --- a/Documentation/devicetree/bindings/sound/samsung,odroid.txt +++ /dev/null @@ -1,54 +0,0 @@ -Samsung Exynos Odroid XU3/XU4 audio complex with MAX98090 codec - -Required properties: - - - compatible - "hardkernel,odroid-xu3-audio" - for Odroid XU3 board, - "hardkernel,odroid-xu4-audio" - for Odroid XU4 board (deprecated), - "samsung,odroid-xu3-audio" - for Odroid XU3 board (deprecated), - "samsung,odroid-xu4-audio" - for Odroid XU4 board (deprecated) - - model - the user-visible name of this sound complex - - clocks - should contain entries matching clock names in the clock-names - property - - samsung,audio-widgets - this property specifies off-codec audio elements - like headphones or speakers, for details see widgets.txt - - samsung,audio-routing - a list of the connections between audio - components; each entry is a pair of strings, the first being the - connection's sink, the second being the connection's source; - valid names for sources and sinks are the MAX98090's pins (as - documented in its binding), and the jacks on the board - - For Odroid X2: - "Headphone Jack", "Mic Jack", "DMIC" - - For Odroid U3, XU3: - "Headphone Jack", "Speakers" - - For Odroid XU4: - no entries - -Required sub-nodes: - - - 'cpu' subnode with a 'sound-dai' property containing the phandle of the I2S - controller - - 'codec' subnode with a 'sound-dai' property containing list of phandles - to the CODEC nodes, first entry must be corresponding to the MAX98090 - CODEC and the second entry must be the phandle of the HDMI IP block node - -Example: - -sound { - compatible = "hardkernel,odroid-xu3-audio"; - model = "Odroid-XU3"; - samsung,audio-routing = - "Headphone Jack", "HPL", - "Headphone Jack", "HPR", - "IN1", "Mic Jack", - "Mic Jack", "MICBIAS"; - - cpu { - sound-dai = <&i2s0 0>; - }; - codec { - sound-dai = <&hdmi>, <&max98090>; - }; -}; diff --git a/Documentation/devicetree/bindings/sound/samsung,odroid.yaml b/Documentation/devicetree/bindings/sound/samsung,odroid.yaml new file mode 100644 index 000000000000..db6d3ea3180e --- /dev/null +++ b/Documentation/devicetree/bindings/sound/samsung,odroid.yaml @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/samsung,odroid.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung Exynos Odroid XU3/XU4 audio complex with MAX98090 codec + +maintainers: + - Krzysztof Kozlowski <krzk(a)kernel.org> + - Sylwester Nawrocki <s.nawrocki(a)samsung.com> + +properties: + compatible: + oneOf: + - const: hardkernel,odroid-xu3-audio + + - const: hardkernel,odroid-xu4-audio + deprecated: true + + - const: samsung,odroid-xu3-audio + deprecated: true + + - const: samsung,odroid-xu4-audio + deprecated: true + + model: + $ref: /schemas/types.yaml#/definitions/string + description: The user-visible name of this sound complex. + + cpu: + type: object + properties: + sound-dai: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: phandles to the I2S controllers + + codec: + type: object + properties: + sound-dai: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: | + List of phandles to the CODEC nodes, + first entry must be corresponding to the MAX98090 CODEC and + the second entry must be the phandle of the HDMI IP block node. + + samsung,audio-widgets: + $ref: /schemas/types.yaml#/definitions/non-unique-string-array + description: | + This property specifies off-codec audio elements + like headphones or speakers, for details see widgets.txt + + samsung,audio-routing: + $ref: /schemas/types.yaml#/definitions/non-unique-string-array + description: | + List of the connections between audio + components; each entry is a pair of strings, the first being the + connection's sink, the second being the connection's source; + valid names for sources and sinks are the MAX98090's pins (as + documented in its binding), and the jacks on the board. + For Odroid X2: "Headphone Jack", "Mic Jack", "DMIC" + For Odroid U3, XU3: "Headphone Jack", "Speakers" + For Odroid XU4: no entries + +required: + - compatible + - model + - cpu + - codec + +examples: + - | + sound { + compatible = "hardkernel,odroid-xu3-audio"; + model = "Odroid-XU3"; + samsung,audio-routing = + "Headphone Jack", "HPL", + "Headphone Jack", "HPR", + "IN1", "Mic Jack", + "Mic Jack", "MICBIAS"; + + cpu { + sound-dai = <&i2s0 0>; + }; + + codec { + sound-dai = <&hdmi>, <&max98090>; + }; + }; + -- 2.17.1

2 1

[alsa-devel] [PATCH] dt-bindings: sound: Convert Samsung Exynos5433 TM2(E) audio complex with WM5110 codec to dt-schema
by Marek Szyprowski 26 Sep '19

26 Sep '19

From: Maciej Falkowski <m.falkowski(a)samsung.com> Convert Samsung Exynos5433 TM2(E) audio complex with WM5110 codec to newer dt-schema format. Signed-off-by: Maciej Falkowski <m.falkowski(a)samsung.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> --- .../bindings/sound/samsung,tm2-audio.txt | 42 ---------- .../bindings/sound/samsung,tm2-audio.yaml | 83 +++++++++++++++++++ 2 files changed, 83 insertions(+), 42 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/samsung,tm2-audio.txt create mode 100644 Documentation/devicetree/bindings/sound/samsung,tm2-audio.yaml diff --git a/Documentation/devicetree/bindings/sound/samsung,tm2-audio.txt b/Documentation/devicetree/bindings/sound/samsung,tm2-audio.txt deleted file mode 100644 index f5ccc12ddc00..000000000000 --- a/Documentation/devicetree/bindings/sound/samsung,tm2-audio.txt +++ /dev/null @@ -1,42 +0,0 @@ -Samsung Exynos5433 TM2(E) audio complex with WM5110 codec - -Required properties: - - - compatible : "samsung,tm2-audio" - - model : the user-visible name of this sound complex - - audio-codec : the first entry should be phandle of the wm5110 audio - codec node, as described in ../mfd/arizona.txt; - the second entry should be phandle of the HDMI - transmitter node - - i2s-controller : the list of phandle and argument tuples pointing to - I2S controllers, the first entry should be I2S0 and - the second one I2S1 - - audio-amplifier : the phandle of the MAX98504 amplifier - - samsung,audio-routing : a list of the connections between audio components; - each entry is a pair of strings, the first being the - connection's sink, the second being the connection's - source; valid names for sources and sinks are the - WM5110's and MAX98504's pins and the jacks on the - board: HP, SPK, Main Mic, Sub Mic, Third Mic, - Headset Mic - - mic-bias-gpios : GPIO pin that enables the Main Mic bias regulator - - -Example: - -sound { - compatible = "samsung,tm2-audio"; - audio-codec = <&wm5110>, <&hdmi>; - i2s-controller = <&i2s0 0>, <&i2s1 0>; - audio-amplifier = <&max98504>; - mic-bias-gpios = <&gpr3 2 0>; - model = "wm5110"; - samsung,audio-routing = - "HP", "HPOUT1L", - "HP", "HPOUT1R", - "SPK", "SPKOUT", - "SPKOUT", "HPOUT2L", - "SPKOUT", "HPOUT2R", - "Main Mic", "MICBIAS2", - "IN1R", "Main Mic"; -}; diff --git a/Documentation/devicetree/bindings/sound/samsung,tm2-audio.yaml b/Documentation/devicetree/bindings/sound/samsung,tm2-audio.yaml new file mode 100644 index 000000000000..377f8cbe17b8 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/samsung,tm2-audio.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/samsung,tm2-audio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung Exynos SoC Exynos5433 TM2(E) audio complex with WM5110 codec + +maintainers: + - Krzysztof Kozlowski <krzk(a)kernel.org> + - Sylwester Nawrocki <s.nawrocki(a)samsung.com> + +properties: + compatible: + const: samsung,tm2-audio + + model: + maxItems: 1 + + audio-codec: + allOf: + - $ref: /schemas/types.yaml#/definitions/phandle-array + - items: + - description: | + phandle of the wm5110 audio codec node, + as described in ../mfd/arizona.txt; + - description: phandle of the HDMI transmitter node. + + i2s-controller: + allOf: + - $ref: /schemas/types.yaml#/definitions/phandle-array + - items: + - description: phandle of the I2S0. + - description: phandle of the I2S1. + + audio-amplifier: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the MAX98504 amplifier. + + samsung,audio-routing: + $ref: /schemas/types.yaml#/definitions/non-unique-string-array + description: | + List of the connections between audio components. + Each entry is a pair of strings, the first being the + connection's sink, the second being the connection's + source. Valid names for sources and sinks are + the WM5110's and MAX98504's pins and the jacks on the + board: HP, SPK, Main Mic, Sub Mic, Third Mic, Headset Mic. + + mic-bias-gpios: + description: GPIO pin that enables the Main Mic bias regulator. + maxItems: 1 + +required: + - compatible + - model + - audio-codec + - i2s-controller + - audio-amplifier + - samsung,audio-routing + - mic-bias-gpios + +additionalProperties: false + +examples: + - | + sound { + compatible = "samsung,tm2-audio"; + model = "wm5110"; + audio-codec = <&wm5110>, <&hdmi>; + i2s-controller = <&i2s0 0>, <&i2s1 0>; + audio-amplifier = <&max98504>; + mic-bias-gpios = <&gpr3>; + samsung,audio-routing = + "HP", "HPOUT1L", + "HP", "HPOUT1R", + "SPK", "SPKOUT", + "SPKOUT", "HPOUT2L", + "SPKOUT", "HPOUT2R", + "Main Mic", "MICBIAS2", + "IN1R", "Main Mic"; + }; + -- 2.17.1

3 3

Re: [alsa-devel] [PATCH V2 1/2] string: Add stracpy and stracpy_pad mechanisms
by Rasmus Villemoes 26 Sep '19

26 Sep '19

On 26/09/2019 10.25, Stephen Kitt wrote: > Le 26/09/2019 09:29, Rasmus Villemoes a écrit : >> On 26/09/2019 02.01, Stephen Kitt wrote: >>> Le 25/09/2019 23:50, Andrew Morton a écrit : >>>> On Tue, 23 Jul 2019 06:51:36 -0700 Joe Perches <joe(a)perches.com> wrote: >>>> >> >> Please don't. At least not for the cases where the source is a string >> literal - that just gives worse code generation (because gcc doesn't >> know anything about strscpy or strlcpy), and while a run-time (silent) >> truncation is better than a run-time buffer overflow, wouldn't it be >> even better with a build time error? > > Yes, that was the plan once Joe's patch gets merged (if it does), and my > patch was only an example of using stracpy, as a step on the road. I was > intending to follow up with a patch converting stracpy to something like > https://www.openwall.com/lists/kernel-hardening/2019/07/06/14 > > __FORTIFY_INLINE ssize_t strscpy(char *dest, const char *src, size_t count) > { > size_t dest_size = __builtin_object_size(dest, 0); > size_t src_size = __builtin_object_size(src, 0); > if (__builtin_constant_p(count) && > __builtin_constant_p(src_size) && > __builtin_constant_p(dest_size) && Eh? Isn't the output of __builtin_object_size() by definition a compile-time constant - whatever the compiler happens to know about the object size (or a sentinel 0 or -1 depending on the type argument)? > > #define stracpy(dest, src) \ > ({ \ > size_t count = ARRAY_SIZE(dest); \ > size_t dest_size = __builtin_object_size(dest, 0); \ > size_t src_size = __builtin_object_size(src, 0); \ > BUILD_BUG_ON(!(__same_type(dest, char[]) || \ > __same_type(dest, unsigned char[]) || \ > __same_type(dest, signed char[]))); \ > \ > (__builtin_constant_p(count) && \ > __builtin_constant_p(src_size) && \ > __builtin_constant_p(dest_size) && \ > src_size <= count && \ > src_size <= dest_size && \ > src[src_size - 1] == '\0') ? \ > (((size_t) strcpy(dest, src)) & 0) + src_size - 1 \ > : \ > strscpy(dest, src, count); \ > }) > > and both of these get optimised to movs when copying a constant string > which fits in the target. But does it catch the case of overflowing a char[] member in a struct passed by reference at build time? I'm surprised that __builtin_object_size(dest, 0) seems to be (size_t)-1, when dest is s->name (with struct s { char name[4]; };). So I'm not very confident that any of the fancy fortify logic actually works here - we _really_ should have some Kbuild infrastructure for saying "this .c file should not compile" so we can test that the fortifications actually work in the simple and common cases. > I was going at this from the angle of improving the existing APIs and > their resulting code. I'm not against stracpy() as a wrapper for strscpy(), but we should make sure that whenever we can fail at build time (i.e., both source and dst lengths known), we do - and in that case also let the compiler optimize the copy (not only to do the immediate movs, but that also gives it wider opportunity to remove it completely as dead stores if the surrounding code ends up dead - with a call to some strscpy(), gcc cannot eliminate that). If stracpy() can be made sufficiently magic that it fails at build time for the string literal cases, fine, let's not add yet another API. Otherwise, I think the static_strcpy() is a much more readable and reliable API for those cases. If I'm reading your stracpy() macro correctly, you're explicitly requesting a run-time truncation (the src_size <= dest_size check causing as to fall back to strscpy) rather than failing at build time. Rasmus

1 0

Re: [alsa-devel] [PATCH V2 1/2] string: Add stracpy and stracpy_pad mechanisms
by Rasmus Villemoes 26 Sep '19

26 Sep '19

On 26/09/2019 02.01, Stephen Kitt wrote: > Le 25/09/2019 23:50, Andrew Morton a écrit : >> On Tue, 23 Jul 2019 06:51:36 -0700 Joe Perches <joe(a)perches.com> wrote: >> >>> Several uses of strlcpy and strscpy have had defects because the >>> last argument of each function is misused or typoed. >>> >>> Add macro mechanisms to avoid this defect. >>> >>> stracpy (copy a string to a string array) must have a string >>> array as the first argument (dest) and uses sizeof(dest) as the >>> count of bytes to copy. >>> >>> These mechanisms verify that the dest argument is an array of >>> char or other compatible types like u8 or s8 or equivalent. >>> >>> A BUILD_BUG is emitted when the type of dest is not compatible. >>> >> >> I'm still reluctant to merge this because we don't have code in -next >> which *uses* it. You did have a patch for that against v1, I believe? >> Please dust it off and send it along? > > Joe had a Coccinelle script to mass-convert strlcpy and strscpy. > Here's a different patch which converts some of ALSA's strcpy calls to > stracpy: Please don't. At least not for the cases where the source is a string literal - that just gives worse code generation (because gcc doesn't know anything about strscpy or strlcpy), and while a run-time (silent) truncation is better than a run-time buffer overflow, wouldn't it be even better with a build time error? Something like /** static_strcpy - run-time version of static initialization * * @d: destination array, must be array of char of known size * @s: source, must be a string literal * * This is a simple wrapper for strcpy(d, s), which checks at build-time that the strcpy() won't overflow. In most cases (for short strings), gcc won't even emit a call to strcpy but rather just do a few immediate stores into the destination, e.g. 0: c7 07 66 6f 6f 00 movl $0x6f6f66,(%rdi) * for strcpy(d->name, "foo"). */ #define static_strcpy(d, s) ({ \ static_assert(__same_type(d, char[]), "destination must be char array"); \ static_assert(__same_type(s, const char[], "source must be a string literal"); \ static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \ strcpy(d, s); \ }) The "" s "" trick guarantees that s is a string literal - it probably doesn't give a very nice error message, but that's why I added the redundant type comparison to a const char[] which should hopefully give a better clue. Rasmus PS: Yes, we already have a fortified strcpy(), but for some reason it doesn't catch the common case where we're populating a char[] member of some struct. So this diff --git a/lib/vsprintf.c b/lib/vsprintf.c index e78017a3e1bd..3b0c5ae9f49e 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -3420,3 +3420,14 @@ int sscanf(const char *buf, const char *fmt, ...) return i; } EXPORT_SYMBOL(sscanf); + +struct s { char name[4]; }; +char buf[4]; +void f3(void) +{ + strcpy(buf, "long"); +} +void f4(struct s *s) +{ + strcpy(s->name, "long"); +} with CONFIG_FORTIFY_SOURCE=y complains about f3(), but f4() is just fine... PPS: A quick test of static_strcpy(): // ss.c #include <errno.h> #include <string.h> #include <assert.h> #define __same_type(x, y) __builtin_types_compatible_p(typeof(x), typeof(y)) #define static_strcpy(d, s) ({ \ static_assert(__same_type(d, char[]), "destination must be char array"); \ static_assert(__same_type(s, const char[]), "source must be a string literal"); \ static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \ strcpy(d, s); \ }) struct s { char name[4]; }; void f0(struct s *s) { static_strcpy(s->name, "foo"); } #if 1 void f1(struct s *s) { static_strcpy(s->name, strerror(EIO)); } void f2(struct s *s) { static_strcpy(s->name, "long"); } void f3(char *d) { static_strcpy(d, "foo"); } void f4(char *d) { static_strcpy(d, strerror(EIO)); } #endif // gcc -Wall -O2 -o ss.o -c ss.c In file included from ss.c:3:0: ss.c: In function ‘f1’: ss.c:9:3: error: static assertion failed: "source must be a string literal" static_assert(__same_type(s, const char[]), "source must be a string literal"); \ ^ ss.c:18:24: note: in expansion of macro ‘static_strcpy’ void f1(struct s *s) { static_strcpy(s->name, strerror(EIO)); } ^~~~~~~~~~~~~ ss.c:18:47: error: expected ‘)’ before ‘strerror’ void f1(struct s *s) { static_strcpy(s->name, strerror(EIO)); } ^ ss.c:10:40: note: in definition of macro ‘static_strcpy’ static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \ ^ In file included from ss.c:3:0: ss.c: In function ‘f2’: ss.c:10:3: error: static assertion failed: "source does not fit in destination" static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \ ^ ss.c:19:24: note: in expansion of macro ‘static_strcpy’ void f2(struct s *s) { static_strcpy(s->name, "long"); } ^~~~~~~~~~~~~ ss.c: In function ‘f3’: ss.c:8:3: error: static assertion failed: "destination must be char array" static_assert(__same_type(d, char[]), "destination must be char array"); \ ^ ss.c:20:20: note: in expansion of macro ‘static_strcpy’ void f3(char *d) { static_strcpy(d, "foo"); } ^~~~~~~~~~~~~ ss.c: In function ‘f4’: ss.c:8:3: error: static assertion failed: "destination must be char array" static_assert(__same_type(d, char[]), "destination must be char array"); \ ^ ss.c:21:20: note: in expansion of macro ‘static_strcpy’ void f4(char *d) { static_strcpy(d, strerror(EIO)); } ^~~~~~~~~~~~~ ss.c:9:3: error: static assertion failed: "source must be a string literal" static_assert(__same_type(s, const char[]), "source must be a string literal"); \ ^ ss.c:21:20: note: in expansion of macro ‘static_strcpy’ void f4(char *d) { static_strcpy(d, strerror(EIO)); } ^~~~~~~~~~~~~ ss.c:21:37: error: expected ‘)’ before ‘strerror’ void f4(char *d) { static_strcpy(d, strerror(EIO)); } ^ ss.c:10:40: note: in definition of macro ‘static_strcpy’ static_assert(sizeof(d) >= sizeof("" s ""), "source does not fit in destination"); \ ^

1 0