| Message ID | 20210916093350.1410403-4-yukuai3@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | handle unexpected message from server | expand |
On 2021/09/16 17:33, Yu Kuai wrote: > The sock that clent send request in nbd_send_cmd() and receive reply > in nbd_read_stat() should be the same. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/block/nbd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 614c6ab2b8fe..c724a5bd7fa4 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) > ret = -ENOENT; > goto out; > } > + if (cmd->index != index) { > + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", > + tag, index, cmd->index); > + } > if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { > dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", > req, cmd->cmd_cookie, nbd_handle_to_cookie(handle)); > Hi, Ming Any suggestions about this patch? Thanks, Kuai
On Sun, Sep 19, 2021 at 06:34:28PM +0800, yukuai (C) wrote: > On 2021/09/16 17:33, Yu Kuai wrote: > > The sock that clent send request in nbd_send_cmd() and receive reply > > in nbd_read_stat() should be the same. > > > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > > --- > > drivers/block/nbd.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > index 614c6ab2b8fe..c724a5bd7fa4 100644 > > --- a/drivers/block/nbd.c > > +++ b/drivers/block/nbd.c > > @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) > > ret = -ENOENT; > > goto out; > > } > > + if (cmd->index != index) { > > + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", > > + tag, index, cmd->index); > > + } > > if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { > > dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", > > req, cmd->cmd_cookie, nbd_handle_to_cookie(handle)); > > > > Hi, Ming > > Any suggestions about this patch? I think this one relies on nbd protocol between server and client, and does the protocol require both request and reply xmitted via same socket? Thanks, Ming
On Wed, Sep 22, 2021 at 05:22:07PM +0800, Ming Lei wrote: > > I think this one relies on nbd protocol between server and client, and > does the protocol require both request and reply xmitted via same > socket? Yes, a reply must be transmitted on the same socket as the request came over. This is because independent sockets are not required to use distinct 64-bit handles, and there is no way for a server to tell if independent clients are related to one another; sending a reply on the wrong socket is thus not guaranteed to reach the intended client. Thus, a compliant server will never send a reply over a different socket than the original request, and if a client ever gets a reply with a handle it was not expecting, then the server is buggy or malicious.
On 2021/09/22 17:22, Ming Lei wrote: > On Sun, Sep 19, 2021 at 06:34:28PM +0800, yukuai (C) wrote: >> On 2021/09/16 17:33, Yu Kuai wrote: >>> The sock that clent send request in nbd_send_cmd() and receive reply >>> in nbd_read_stat() should be the same. >>> >>> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >>> --- >>> drivers/block/nbd.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c >>> index 614c6ab2b8fe..c724a5bd7fa4 100644 >>> --- a/drivers/block/nbd.c >>> +++ b/drivers/block/nbd.c >>> @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) >>> ret = -ENOENT; >>> goto out; >>> } >>> + if (cmd->index != index) { >>> + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", >>> + tag, index, cmd->index); >>> + } >>> if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { >>> dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", >>> req, cmd->cmd_cookie, nbd_handle_to_cookie(handle)); >>> >> >> Hi, Ming >> >> Any suggestions about this patch? > > I think this one relies on nbd protocol between server and client, and > does the protocol require both request and reply xmitted via same > socket? > I searched nbd-server source code, and found that socket_read() and send_reply->socket_write() are always come in pares and using the same socket. BTW, if server reply a read request from a unexpected sock, then nbd_read_stat() might stuck in receiving the read data. And for worse, nbd_read_stat() can mistake the normal reply message for the read data afterwards and corrupt client. Thanks, Kuai
On Wed, Sep 22, 2021 at 05:22:07PM +0800, Ming Lei wrote: > On Sun, Sep 19, 2021 at 06:34:28PM +0800, yukuai (C) wrote: > > On 2021/09/16 17:33, Yu Kuai wrote: > > > The sock that clent send request in nbd_send_cmd() and receive reply > > > in nbd_read_stat() should be the same. > > > > > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > > > --- > > > drivers/block/nbd.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > > index 614c6ab2b8fe..c724a5bd7fa4 100644 > > > --- a/drivers/block/nbd.c > > > +++ b/drivers/block/nbd.c > > > @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) > > > ret = -ENOENT; > > > goto out; > > > } > > > + if (cmd->index != index) { > > > + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", > > > + tag, index, cmd->index); > > > + } > > > if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { > > > dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", > > > req, cmd->cmd_cookie, nbd_handle_to_cookie(handle)); > > > > > > > Hi, Ming > > > > Any suggestions about this patch? > > I think this one relies on nbd protocol between server and client, and > does the protocol require both request and reply xmitted via same > socket? As Eric already answered: yes, the request and reply are specified such that they must be transmitted over the same socket. For more details, you can find the protocol specification at https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md Please note that the protocol defined there has some options that are not currently supported by the Linux nbd implementation -- specifically the "structured reply" message format (and all that requires it) is not implemented (yet?).
On Thu, Sep 16, 2021 at 05:33:46PM +0800, Yu Kuai wrote: > The sock that clent send request in nbd_send_cmd() and receive reply > in nbd_read_stat() should be the same. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/block/nbd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 614c6ab2b8fe..c724a5bd7fa4 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) > ret = -ENOENT; > goto out; > } > + if (cmd->index != index) { > + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", > + tag, index, cmd->index); > + } > if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { > dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", > req, cmd->cmd_cookie, nbd_handle_to_cookie(handle)); > -- > 2.31.1 > Reviewed-by: Ming Lei <ming.lei@redhat.com>
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 614c6ab2b8fe..c724a5bd7fa4 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -746,6 +746,10 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index) ret = -ENOENT; goto out; } + if (cmd->index != index) { + dev_err(disk_to_dev(nbd->disk), "Unexpected reply %d from different sock %d (expected %d)", + tag, index, cmd->index); + } if (cmd->cmd_cookie != nbd_handle_to_cookie(handle)) { dev_err(disk_to_dev(nbd->disk), "Double reply on req %p, cmd_cookie %u, handle cookie %u\n", req, cmd->cmd_cookie, nbd_handle_to_cookie(handle));
The sock that clent send request in nbd_send_cmd() and receive reply in nbd_read_stat() should be the same. Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- drivers/block/nbd.c | 4 ++++ 1 file changed, 4 insertions(+)