lkml.org 
[lkml]   [2021]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] [v13] wireless: Initial driver submission for pureLiFi STA devices
    From
    Date
    On Fri, 2021-02-19 at 05:15 +0000, Srinivasan Raju wrote:
    > Hi,
    >
    > Please find a few responses to the comments , We will fix rest of the comments and submit v14 of the patch.
    >
    > > Also, you *really* need some validation here, rather than blindly
    > > trusting that the file is well-formed, otherwise you immediately have a
    > > security issue here.
    >
    > The firmware is signed and the harware validates the signature , so we are not validating in the software.

    That wasn't my point. My point was that the kernel code trusts the
    validity of the firmware image, in the sense of e.g. this piece:

    > + no_of_files = *(u32 *)&fw_packed->data[0];

    If the firmware file was corrupted (intentionally/maliciously or not), this could now be say 0xffffffff.

    > + for (step = 0; step < no_of_files; step++) {

    > + start_addr = *(u32 *)&fw_packed->data[4 + (step * 4)];

    But you trust it completely and don't even check that "4 + (step * 4)"
    fits into the length of the data!

    That's what I meant. Don't allow this to crash the kernel.

    > > > +static const struct plf_reg_alpha2_map reg_alpha2_map[] = {
    > > > + { PLF_REGDOMAIN_FCC, "US" },
    > > > + { PLF_REGDOMAIN_IC, "CA" },
    > > > + { PLF_REGDOMAIN_ETSI, "DE" }, /* Generic ETSI, use most restrictive */
    > > > + { PLF_REGDOMAIN_JAPAN, "JP" },
    > > > + { PLF_REGDOMAIN_SPAIN, "ES" },
    > > > + { PLF_REGDOMAIN_FRANCE, "FR" },
    > > > +};
    > > You actually have regulatory restrictions on this stuff?
    >
    > Currently, There are no regulatory restrictions applicable for LiFi ,
    > regulatory_hint setting is only for wifi core

    So why do you have PLF_REGDOMAIN_* things? What does that even mean
    then?

    > > OTOH, I can sort of see why/how you're reusing wifi functionality here,
    > > very old versions of wifi even had an infrared PHY I think.
    > >
    > > Conceptually, it seems odd. Perhaps we should add a new band definition?
    > >
    > > And what I also asked above - how much of the rate stuff is completely
    > > fake? Are you really doing CCK/OFDM in some (strange?) way?
    >
    > Yes, your understanding is correct, and we use OFDM.
    > For now we will use the existing band definition.

    OFDM over infrared, nice.

    Still, I'm not convinced we should piggy-back this on 2.4 GHz.

    NL80211_BAND_1THZ? ;-)

    Ok, I don't know what wavelength you're using, of course...

    But at least thinking about this, wouldn't we be better off if we define
    NL80211_BAND_INFRARED or something? That way, at least we can tell that
    it's not going to interoperate with real 2.4 GHz, etc.

    OTOH, this is a bit of a slippery slow - what if somebody else designs
    an *incompatible* infrared PHY? I guess this is not really standardised
    at this point.

    Not really sure about all this, but I guess we need to think about it
    more.

    What are your reasons for piggy-backing on 2.4 GHz? Just practical "it's
    there and we don't care"?

    johannes

    \
     
     \ /
      Last update: 2021-02-19 09:27    [W:3.519 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site