netdev - Re: [PATCH net v2] net: ipa: properly limit modem routing table use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Sep 2022 08:14:00 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Alex Elder <elder@...aro.org>
Cc:     davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
        mka@...omium.org, evgreen@...omium.org, bjorn.andersson@...aro.org,
        quic_cpratapa@...cinc.com, quic_avuyyuru@...cinc.com,
        quic_jponduru@...cinc.com, quic_subashab@...cinc.com,
        elder@...nel.org, netdev@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: ipa: properly limit modem routing table use

On Tue, 13 Sep 2022 15:46:02 -0500 Alex Elder wrote:
> IPA can route packets between IPA-connected entities.  The AP and
> modem are currently the only such entities supported, and no routing
> is required to transfer packets between them.
> 
> The number of entries in each routing table is fixed, and defined at
> initialization time.  Some of these entries are designated for use
> by the modem, and the rest are available for the AP to use.  The AP
> sends a QMI message to the modem which describes (among other
> things) information about routing table memory available for the
> modem to use.
> 
> Currently the QMI initialization packet gives wrong information in
> its description of routing tables.  What *should* be supplied is the
> maximum index that the modem can use for the routing table memory
> located at a given location.  The current code instead supplies the
> total *number* of routing table entries.  Furthermore, the modem is
> granted the entire table, not just the subset it's supposed to use.
> 
> This patch fixes this.  First, the ipa_mem_bounds structure is
> generalized so its "end" field can be interpreted either as a final
> byte offset, or a final array index.  Second, the IPv4 and IPv6
> (non-hashed and hashed) table information fields in the QMI
> ipa_init_modem_driver_req structure are changed to be ipa_mem_bounds
> rather than ipa_mem_array structures.  Third, we set the "end" value
> for each routing table to be the last index, rather than setting the
> "count" to be the number of indices.  Finally, instead of allowing
> the modem to use all of a routing table's memory, it is limited to
> just the portion meant to be used by the modem.  In all versions of
> IPA currently supported, that is IPA_ROUTE_MODEM_COUNT (8) entries.
> 
> Update a few comments for clarity.
> 
> Fixes: 530f9216a9537 ("soc: qcom: ipa: AP/modem communications")
> Signed-off-by: Alex Elder <elder@...aro.org>
> ---
> v2: Update the element info arrays defining the modified QMI message
>     so it uses the ipa_mem_bounds_ei structure rather than the
>     ipa_mem_array_ei structure.  The message format is identical,
>     but the code was incorrect without that change.

Unclear to me why, ipa_mem_bounds_ei and ipa_mem_array_ei seem
identical, other than s/end/count/. Overall the patch feels 
a touch too verbose for a fix, makes it harder to grasp the key
functional change IMHO. I could be misunderstanding but please
keep the goal of making fixes small and crisp in mind for the future.