| Message ID | 577E1FAD02000078000FBD88@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/07/16 08:23, Jan Beulich wrote: > Inability to locate a user mode specified transaction ID should not > lead to a kernel crash. For other than XS_TRANSACTION_START also > don't issue anything to xenbus if the specified ID doesn't match that > of any active transaction. Applied to for-linus-4.7b and Cc'd to stable, thanks. David
Hi, > --- 4.7-rc6-xen.orig/drivers/xen/xenbus/xenbus_dev_frontend.c > +++ 4.7-rc6-xen/drivers/xen/xenbus/xenbus_dev_frontend.c > @@ -316,11 +316,18 @@ static int xenbus_write_transaction(unsi > rc = -ENOMEM; > goto out; > } > + } else { > + list_for_each_entry(trans, &u->transactions, list) > + if (trans->handle.id == u->u.msg.tx_id) > + break; > + if (&trans->list == &u->transactions) > + return -ESRCH; > } Shouldn't there be some tolerance in there in case the tx_id is zero ? (i.e. no transaction). I'm trying to find out why just doing "xenstore-ls" doesn't work on my 4.4.20 kernel and when stracing it, I see it doing : access("/dev/xen/xenbus", F_OK) = 0 stat("/dev/xen/xenbus", {st_mode=S_IFCHR|0600, st_rdev=makedev(10, 60), ...}) = 0 open("/dev/xen/xenbus", O_RDWR) = 3 brk(0) = 0x18e4000 brk(0x1905000) = 0x1905000 rt_sigaction(SIGPIPE, {SIG_IGN, [], SA_RESTORER, 0x7fe4dd98e0e0}, {SIG_DFL, [], 0}, 8) = 0 write(3, "\1\0\0\0\0\0\0\0\0\0\0\0\2\0\0\0", 16) = 16 write(3, "/\0", 2) = -1 ESRCH (No such process) So either what xenstore-ls does is invalid, or that condition requiring a transaction is too strict. Or am I missing something here ? Cheers, Sylvain Munaut
## Sylvain Munaut (s.munaut@whatever-company.com): > I'm trying to find out why just doing "xenstore-ls" doesn't work on my > 4.4.20 kernel and when stracing it, I see it doing : That looks like the same brokeness I reported earlier: https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg02496.html Luckily I'm not alone with that :) Regards, Christoph
>>> On 21.08.16 at 21:36, <s.munaut@whatever-company.com> wrote: >> --- 4.7-rc6-xen.orig/drivers/xen/xenbus/xenbus_dev_frontend.c >> +++ 4.7-rc6-xen/drivers/xen/xenbus/xenbus_dev_frontend.c >> @@ -316,11 +316,18 @@ static int xenbus_write_transaction(unsi >> rc = -ENOMEM; >> goto out; >> } >> + } else { >> + list_for_each_entry(trans, &u->transactions, list) >> + if (trans->handle.id == u->u.msg.tx_id) >> + break; >> + if (&trans->list == &u->transactions) >> + return -ESRCH; >> } > > Shouldn't there be some tolerance in there in case the tx_id is zero ? > (i.e. no transaction). > > I'm trying to find out why just doing "xenstore-ls" doesn't work on my > 4.4.20 kernel and when stracing it, I see it doing : > > access("/dev/xen/xenbus", F_OK) = 0 > stat("/dev/xen/xenbus", {st_mode=S_IFCHR|0600, st_rdev=makedev(10, > 60), ...}) = 0 > open("/dev/xen/xenbus", O_RDWR) = 3 > brk(0) = 0x18e4000 > brk(0x1905000) = 0x1905000 > rt_sigaction(SIGPIPE, {SIG_IGN, [], SA_RESTORER, 0x7fe4dd98e0e0}, > {SIG_DFL, [], 0}, 8) = 0 > write(3, "\1\0\0\0\0\0\0\0\0\0\0\0\2\0\0\0", 16) = 16 > write(3, "/\0", 2) = -1 ESRCH (No such process) > > > So either what xenstore-ls does is invalid, or that condition > requiring a transaction is too strict. > > Or am I missing something here ? See https://patchwork.kernel.org/patch/9281193/. Jan
Hi Jan,
> See https://patchwork.kernel.org/patch/9281193/.
Thanks for the pointer !
I had checked the kernel git tree for a potential fix, but didn't
think of patchwork.
Cheers,
Sylvain Munaut
--- 4.7-rc6-xen.orig/drivers/xen/xenbus/xenbus_dev_frontend.c +++ 4.7-rc6-xen/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -316,11 +316,18 @@ static int xenbus_write_transaction(unsi rc = -ENOMEM; goto out; } + } else { + list_for_each_entry(trans, &u->transactions, list) + if (trans->handle.id == u->u.msg.tx_id) + break; + if (&trans->list == &u->transactions) + return -ESRCH; } reply = xenbus_dev_request_and_reply(&u->u.msg); if (IS_ERR(reply)) { - kfree(trans); + if (msg_type == XS_TRANSACTION_START) + kfree(trans); rc = PTR_ERR(reply); goto out; } @@ -333,12 +340,7 @@ static int xenbus_write_transaction(unsi list_add(&trans->list, &u->transactions); } } else if (u->u.msg.type == XS_TRANSACTION_END) { - list_for_each_entry(trans, &u->transactions, list) - if (trans->handle.id == u->u.msg.tx_id) - break; - BUG_ON(&trans->list == &u->transactions); list_del(&trans->list); - kfree(trans); }
Inability to locate a user mode specified transaction ID should not lead to a kernel crash. For other than XS_TRANSACTION_START also don't issue anything to xenbus if the specified ID doesn't match that of any active transaction. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- drivers/xen/xenbus/xenbus_dev_frontend.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)