All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: netdev@vger.kernel.org
Cc: Sathya Perla <sathya.perla@emulex.com>
Subject: Re: [benet] possible endianness bug in be_cmd_txq_create()
Date: Sun, 10 Dec 2017 18:43:08 +0000	[thread overview]
Message-ID: <20171210184307.GT21978@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20171210164120.GS21978@ZenIV.linux.org.uk>

On Sun, Dec 10, 2017 at 04:41:20PM +0000, Al Viro wrote:

> I don't have the hardware, so the above is completely untested (caught by
> sparse when trying to do endianness annotations in drivers/net), but it
> does look like it might be worth a look from benet maintainers.

Another very fishy place is be_roce_mcc_cmd().

        req = embedded_payload(wrb);
        resp = embedded_payload(wrb);

        be_wrb_cmd_hdr_prepare(req, hdr->subsystem,
                               hdr->opcode, wrb_payload_size, wrb, NULL);

OK, we'd formed a (little-endian) header in the first 32 bytes of req and
filled wrb->{tag0,tag1,payload_length,embedded}.

        memcpy(req, wrb_payload, wrb_payload_size);
        be_dws_cpu_to_le(req, wrb_payload_size);

... only to overwrite *req with wrb_payload bulk-converted to little-endian.
OK, so it's responsibility of caller to have prepare payload so that it
would form a valid header after byteswap.

        status = be_mcc_notify_wait(adapter);
        if (cmd_status)
                *cmd_status = (status & 0xffff);
        if (ext_status)
                *ext_status = 0;

OK, we submit the resulting wrb and wait for reply.

        memcpy(wrb_payload, resp, sizeof(*resp) + resp->response_length);
        be_dws_le_to_cpu(wrb_payload, sizeof(*resp) + resp->response_length);

Then we copy the response back into caller-supplied buffer, bulk-converting
it from little-endian to host-endian.  Hardware puts little-endian there,
caller wants host-endian.  But... ->response_length is a 32bit value at
offset 8 from the beginning of response.  Is it little-endian or host-endian?
In the former case we are fucked on b-e host - the value we pass to memcpy()
is going to be in hundreds of megabytes...  And everything in response seems
to be little-endian - we certainly include response_length itself into the
area covered by that bulk byteswap.

  reply	other threads:[~2017-12-10 18:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-10 16:41 [benet] possible endianness bug in be_cmd_txq_create() Al Viro
2017-12-10 18:43 ` Al Viro [this message]
2017-12-10 20:17   ` Al Viro
2018-01-29  7:33 ` Sathya Perla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171210184307.GT21978@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=sathya.perla@emulex.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.